Skip to content

core: Reduce amount of calls to getattr/Stat, improve vfs.read #157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions core/src/main/java/org/dcache/nfs/v3/MountServer.java
Original file line number Diff line number Diff line change
Expand Up @@ -105,18 +105,18 @@ public mountres3 MOUNTPROC3_MNT_3(RpcCall call$, dirpath arg1) {
try {

Inode rootInode = path2Inode(_vfs, mountPoint);
Stat stat = _vfs.getattr(rootInode);
Stat.Type type = _vfs.getattr(rootInode, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type();

if (stat.type() == Stat.Type.SYMLINK) {
if (type == Stat.Type.SYMLINK) {
/*
* we resolve symlink only once
*/
String path = _vfs.readlink(rootInode);
rootInode = path2Inode(_vfs, path);
stat = _vfs.getattr(rootInode);
type = _vfs.getattr(rootInode, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type();
}

if (stat.type() != Stat.Type.DIRECTORY) {
if (type != Stat.Type.DIRECTORY) {
throw new NotDirException("Path is not a directory");
}

Expand Down
12 changes: 6 additions & 6 deletions core/src/main/java/org/dcache/nfs/v4/OperationLINK.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
result.oplink.resok4.cinfo.atomic = true;

Inode parent = context.currentInode();
Stat parentDirStat = context.getFs().getattr(parent);
Stat inodeStat = context.getFs().getattr(context.savedInode());

if (parentDirStat.type() != Stat.Type.DIRECTORY) {
throw new NotDirException("Can't create a hard-link in non directory object");
Stat.Type inodeStatType = context.getFs().getattr(context.savedInode(), Stat.STAT_ATTRIBUTES_TYPE_ONLY).type();
if (inodeStatType == Stat.Type.DIRECTORY) {
throw new IsDirException("Can't hard-link a directory");
}

if (inodeStat.type() == Stat.Type.DIRECTORY) {
throw new IsDirException("Can't hard-link a directory");
Stat parentDirStat = context.getFs().getattr(parent);
if (parentDirStat.type() != Stat.Type.DIRECTORY) {
throw new NotDirException("Can't create a hard-link in non directory object");
}

result.oplink.resok4.cinfo.before = new changeid4(parentDirStat.getGeneration());
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/org/dcache/nfs/v4/OperationLOCKT.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.dcache.nfs.v4.xdr.offset4;
import org.dcache.nfs.v4.xdr.state_owner4;
import org.dcache.nfs.vfs.Inode;
import org.dcache.nfs.vfs.Stat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -57,7 +58,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti
throw new InvalException("zero lock len");
}

switch (context.getFs().getattr(inode).type()) {
switch (context.getFs().getattr(inode, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type()) {
case REGULAR:
// OK
break;
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/org/dcache/nfs/v4/OperationLOOKUP.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF

String name = NameFilter.convertName(_args.oplookup.objname.value);

Stat stat = context.getFs().getattr(context.currentInode());
if (stat.type() == Stat.Type.SYMLINK) {
Stat.Type statType = context.getFs().getattr(context.currentInode(), Stat.STAT_ATTRIBUTES_TYPE_ONLY).type();
if (statType == Stat.Type.SYMLINK) {
throw new SymlinkException("parent not a symbolic link");
}

if (stat.type() != Stat.Type.DIRECTORY) {
if (statType != Stat.Type.DIRECTORY) {
throw new NotDirException("parent not a directory");
}

Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/org/dcache/nfs/v4/OperationLOOKUPP.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ public OperationLOOKUPP(nfs_argop4 args) {
public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNFSException, IOException {
final LOOKUPP4res res = result.oplookupp;

Stat stat = context.getFs().getattr(context.currentInode());
Stat.Type statType = context.getFs().getattr(context.currentInode(), Stat.STAT_ATTRIBUTES_TYPE_ONLY).type();

if (stat.type() == Stat.Type.SYMLINK) {
if (statType == Stat.Type.SYMLINK) {
throw new SymlinkException("get parent on a symlink");
}

if (stat.type() != Stat.Type.DIRECTORY) {
if (statType != Stat.Type.DIRECTORY) {
throw new NotDirException("not a directory");
}

Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/dcache/nfs/v4/OperationOPEN.java
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,8 @@ private void checkCanAccess(CompoundContext context, Inode inode, uint32_t share
throw new AccessException();
}

Stat stat = context.getFs().getattr(inode);
switch (stat.type()) {
Stat.Type statType = context.getFs().getattr(inode, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type();
switch (statType) {
case REGULAR:
// OK
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import org.dcache.nfs.v4.xdr.nfs_opnum4;
import org.dcache.nfs.v4.xdr.nfs_resop4;
import org.dcache.nfs.v4.xdr.stateid4;
import org.dcache.nfs.vfs.Inode;
import org.dcache.nfs.vfs.Stat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -53,14 +52,13 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti
throw new NotSuppException("operation OPEN_CONFIRM4 is obsolete in 4.x, x > 0");
}

Inode inode = context.currentInode();
Stat stat = context.getFs().getattr(context.currentInode());
Stat.Type statType = context.getFs().getattr(context.currentInode(), Stat.STAT_ATTRIBUTES_TYPE_ONLY).type();

if (stat.type() == Stat.Type.DIRECTORY) {
if (statType == Stat.Type.DIRECTORY) {
throw new IsDirException();
}

if (stat.type() == Stat.Type.SYMLINK) {
if (statType == Stat.Type.SYMLINK) {
throw new InvalException();
}

Expand Down
31 changes: 8 additions & 23 deletions core/src/main/java/org/dcache/nfs/v4/OperationREAD.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@
import java.nio.ByteBuffer;

import org.dcache.nfs.nfsstat;
import org.dcache.nfs.status.InvalException;
import org.dcache.nfs.status.IsDirException;
import org.dcache.nfs.status.NfsIoException;
import org.dcache.nfs.status.OpenModeException;
import org.dcache.nfs.v4.xdr.READ4res;
import org.dcache.nfs.v4.xdr.READ4resok;
Expand All @@ -34,7 +31,6 @@
import org.dcache.nfs.v4.xdr.nfs_opnum4;
import org.dcache.nfs.v4.xdr.nfs_resop4;
import org.dcache.nfs.v4.xdr.stateid4;
import org.dcache.nfs.vfs.Stat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -50,17 +46,8 @@ public OperationREAD(nfs_argop4 args) {
public void process(CompoundContext context, nfs_resop4 result) throws IOException {
final READ4res res = result.opread;

Stat inodeStat = context.getFs().getattr(context.currentInode());
stateid4 stateid = Stateids.getCurrentStateidIfNeeded(context, _args.opread.stateid);

if (inodeStat.type() == Stat.Type.DIRECTORY) {
throw new IsDirException();
}

if (inodeStat.type() == Stat.Type.SYMLINK) {
throw new InvalException();
}

NFS4Client client;
if (context.getMinorversion() == 0) {
/*
Expand All @@ -85,19 +72,17 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti

ByteBuffer buf = ByteBuffer.allocate(count);

int bytesReaded = context.getFs().read(inode, buf, offset);
if (bytesReaded < 0) {
throw new NfsIoException("IO not allowed");
}

buf.flip();
res.status = nfsstat.NFS_OK;
res.resok4 = new READ4resok();
int bytesRead = context.getFs().read(inode, buf, offset, res.resok4::setEOF);

res.resok4.data = buf;

if (offset + bytesReaded >= inodeStat.getSize()) {
if (bytesRead < 0) {
buf.clear();
res.resok4.eof = true;
} else {
buf.flip();
}

res.status = nfsstat.NFS_OK;
res.resok4.data = buf;
}
}
4 changes: 2 additions & 2 deletions core/src/main/java/org/dcache/nfs/v4/OperationREADDIR.java
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ public void process(final CompoundContext context, nfs_resop4 result) throws Chi
throw new BadCookieException("bad cookie : " + startValue);
}

Stat stat = context.getFs().getattr(dir);
if (stat.type() != Stat.Type.DIRECTORY) {
Stat.Type statType = context.getFs().getattr(dir, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type();
if (statType != Stat.Type.DIRECTORY) {
throw new NotDirException();
}

Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/dcache/nfs/v4/OperationREADLINK.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ public OperationREADLINK(nfs_argop4 args) {
public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNFSException, IOException {
final READLINK4res res = result.opreadlink;

Stat stat = context.getFs().getattr(context.currentInode());
if (stat.type() != Stat.Type.SYMLINK) {
Stat.Type statType = context.getFs().getattr(context.currentInode(), Stat.STAT_ATTRIBUTES_TYPE_ONLY).type();
if (statType != Stat.Type.SYMLINK) {
throw new InvalException("not a symlink");
}

Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/dcache/nfs/v4/OperationSECINFO.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti

final SECINFO4res res = result.opsecinfo;
Inode dir = context.currentInode();
Stat stat = context.getFs().getattr(dir);
if (stat.type() != Stat.Type.DIRECTORY) {
Stat.Type statType = context.getFs().getattr(dir, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type();
if (statType != Stat.Type.DIRECTORY) {
throw new NotDirException();
}

Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,14 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF

_args.opwrite.offset.checkOverflow(_args.opwrite.data.remaining(), "offset + length overflow");

Stat stat = context.getFs().getattr(context.currentInode());
Stat.Type statType = context.getFs().getattr(context.currentInode(), Stat.STAT_ATTRIBUTES_TYPE_ONLY).type();
stateid4 stateid = Stateids.getCurrentStateidIfNeeded(context, _args.opwrite.stateid);

if (stat.type() == Stat.Type.DIRECTORY) {
if (statType == Stat.Type.DIRECTORY) {
throw new IsDirException();
}

if (stat.type() == Stat.Type.SYMLINK) {
if (statType == Stat.Type.SYMLINK) {
throw new InvalException("path is a symlink");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
final COMMIT4res res = result.opcommit;
if (context.getFs() != null) {
Inode inode = context.currentInode();
Stat stat = context.getFs().getattr(inode);
Stat.Type statType = context.getFs().getattr(inode, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type();

if (stat.type() == Stat.Type.DIRECTORY) {
if (statType == Stat.Type.DIRECTORY) {
throw new IsDirException("Invalid can't commit a directory");
}

if (stat.type() != Stat.Type.REGULAR) {
if (statType != Stat.Type.REGULAR) {
throw new InvalException("Invalid object type");
}

Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/org/dcache/nfs/v4/ds/DSOperationWRITE.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,13 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti
_args.opwrite.offset.checkOverflow(_args.opwrite.data.remaining(), "offset + length overflow");

Inode inode = context.currentInode();
Stat stat = context.getFs().getattr(inode);
Stat.Type statType = context.getFs().getattr(inode, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type();

if (stat.type() == Stat.Type.DIRECTORY) {
if (statType == Stat.Type.DIRECTORY) {
throw new IsDirException("Can't WRITE into a directory inode");
}

if (stat.type() != Stat.Type.REGULAR) {
if (statType != Stat.Type.REGULAR) {
throw new InvalException("Invalid object type");
}

Expand Down
3 changes: 3 additions & 0 deletions core/src/main/java/org/dcache/nfs/v4/xdr/READ4resok.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,8 @@ public void xdrDecode(XdrDecodingStream xdr)
data = xdr.xdrDecodeByteBuffer();
}

public void setEOF() {
this.eof = true;
}
}
// End of READ4resok.java
12 changes: 12 additions & 0 deletions core/src/main/java/org/dcache/nfs/vfs/ForwardingFileSystem.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@

import java.io.IOException;
import java.nio.ByteBuffer;
import java.util.EnumSet;
import java.util.concurrent.CompletableFuture;

import javax.security.auth.Subject;

import org.dcache.nfs.v4.NfsIdMapping;
import org.dcache.nfs.v4.xdr.nfsace4;
import org.dcache.nfs.vfs.Stat.StatAttribute;

/**
* A file system which forwards all its method calls to another file system. Subclasses should override one or more
Expand Down Expand Up @@ -104,6 +106,11 @@ public int read(Inode inode, ByteBuffer data, long offset) throws IOException {
return delegate().read(inode, data, offset);
}

@Override
public int read(Inode inode, ByteBuffer data, long offset, Runnable eofReached) throws IOException {
return delegate().read(inode, data, offset, eofReached);
}

@Override
public String readlink(Inode inode) throws IOException {
return delegate().readlink(inode);
Expand Down Expand Up @@ -141,6 +148,11 @@ public Stat getattr(Inode inode) throws IOException {
return delegate().getattr(inode);
}

@Override
public Stat getattr(Inode inode, EnumSet<StatAttribute> attributes) throws IOException {
return delegate().getattr(inode, attributes);
}

@Override
public void setattr(Inode inode, Stat stat) throws IOException {
delegate().setattr(inode, stat);
Expand Down
Loading