<feed xmlns='http://www.w3.org/2005/Atom'>
<title>linux-toradex.git/fs/nfs/dir.c, branch v6.2-rc8</title>
<subtitle>Linux kernel for Apalis and Colibri modules</subtitle>
<link rel='alternate' type='text/html' href='https://git.toradex.cn/cgit/linux-toradex.git/'/>
<entry>
<title>NFS: Fix up a sparse warning</title>
<updated>2023-01-02T01:17:26+00:00</updated>
<author>
<name>Trond Myklebust</name>
<email>trond.myklebust@hammerspace.com</email>
</author>
<published>2023-01-02T01:17:23+00:00</published>
<link rel='alternate' type='text/html' href='https://git.toradex.cn/cgit/linux-toradex.git/commit/?id=5e9a7b9c2ea18551759833146a181b14835bfe39'/>
<id>5e9a7b9c2ea18551759833146a181b14835bfe39</id>
<content type='text'>
sparse is warning about an incorrect RCU dereference.
fs/nfs/dir.c:2965:56: warning: incorrect type in argument 1 (different address spaces)
fs/nfs/dir.c:2965:56:    expected struct cred const *
fs/nfs/dir.c:2965:56:    got struct cred const [noderef] __rcu *const cred

Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
sparse is warning about an incorrect RCU dereference.
fs/nfs/dir.c:2965:56: warning: incorrect type in argument 1 (different address spaces)
fs/nfs/dir.c:2965:56:    expected struct cred const *
fs/nfs/dir.c:2965:56:    got struct cred const [noderef] __rcu *const cred

Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>NFS: Judge the file access cache's timestamp in rcu path</title>
<updated>2023-01-02T01:12:47+00:00</updated>
<author>
<name>Chengen Du</name>
<email>chengen.du@canonical.com</email>
</author>
<published>2022-12-30T03:04:32+00:00</published>
<link rel='alternate' type='text/html' href='https://git.toradex.cn/cgit/linux-toradex.git/commit/?id=029085b8949f5d269ae2bbd14915407dd0c7f902'/>
<id>029085b8949f5d269ae2bbd14915407dd0c7f902</id>
<content type='text'>
If the user's login time is newer than the cache's timestamp,
we expect the cache may be stale and need to clear.
The stale cache will remain in the list's tail if no other
users operate on that inode.
Once the user accesses the inode, the stale cache will be
returned in rcu path.

Signed-off-by: Chengen Du &lt;chengen.du@canonical.com&gt;
Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
If the user's login time is newer than the cache's timestamp,
we expect the cache may be stale and need to clear.
The stale cache will remain in the list's tail if no other
users operate on that inode.
Once the user accesses the inode, the stale cache will be
returned in rcu path.

Signed-off-by: Chengen Du &lt;chengen.du@canonical.com&gt;
Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>NFS: Trigger the "ls -l" readdir heuristic sooner</title>
<updated>2022-11-28T03:09:59+00:00</updated>
<author>
<name>Benjamin Coddington</name>
<email>bcodding@redhat.com</email>
</author>
<published>2022-09-20T17:00:21+00:00</published>
<link rel='alternate' type='text/html' href='https://git.toradex.cn/cgit/linux-toradex.git/commit/?id=85aa8ddc3818718208c3cfdfda9c8c908c9dead1'/>
<id>85aa8ddc3818718208c3cfdfda9c8c908c9dead1</id>
<content type='text'>
Since commit 1a34c8c9a49e ("NFS: Support larger readdir buffers") has
updated dtsize, and with recent improvements to the READDIRPLUS helper
heuristic, the heuristic may not trigger until many dentries are emitted
to userspace.   This will cause many thousands of GETATTR calls for "ls
-l" when the directory's pagecache has already been populated.  This
manifests as poor performance for long directory listings after an
initially fast "ls -l".

Fix this by emitting only 17 entries for any first pass through the NFS
directory's -&gt;iterate_shared(), which allows userpace to prime the
counters for the heuristic.

Signed-off-by: Benjamin Coddington &lt;bcodding@redhat.com&gt;
Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Since commit 1a34c8c9a49e ("NFS: Support larger readdir buffers") has
updated dtsize, and with recent improvements to the READDIRPLUS helper
heuristic, the heuristic may not trigger until many dentries are emitted
to userspace.   This will cause many thousands of GETATTR calls for "ls
-l" when the directory's pagecache has already been populated.  This
manifests as poor performance for long directory listings after an
initially fast "ls -l".

Fix this by emitting only 17 entries for any first pass through the NFS
directory's -&gt;iterate_shared(), which allows userpace to prime the
counters for the heuristic.

Signed-off-by: Benjamin Coddington &lt;bcodding@redhat.com&gt;
Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>NFS: Clear the file access cache upon login</title>
<updated>2022-11-28T03:09:59+00:00</updated>
<author>
<name>Trond Myklebust</name>
<email>trond.myklebust@hammerspace.com</email>
</author>
<published>2022-08-26T23:44:44+00:00</published>
<link rel='alternate' type='text/html' href='https://git.toradex.cn/cgit/linux-toradex.git/commit/?id=0eb43812c0270ee3d005ff32f91f7d0a6c4943af'/>
<id>0eb43812c0270ee3d005ff32f91f7d0a6c4943af</id>
<content type='text'>
POSIX typically only refreshes the user's supplementary group
information upon login. Since NFS servers may often refresh their
concept of the user supplementary group membership at their own cadence,
it is possible for the NFS client's access cache to become stale due to
the user's group membership changing on the server after the user has
already logged in on the client.
While it is reasonable to expect that such group membership changes are
rare, and that we do not want to optimise the cache to accommodate them,
it is also not unreasonable for the user to expect that if they log out
and log back in again, that the staleness would clear up.

Reviewed-by: Benjamin Coddington &lt;bcodding@redhat.com&gt;
Tested-by: Benjamin Coddington &lt;bcodding@redhat.com&gt;
Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
POSIX typically only refreshes the user's supplementary group
information upon login. Since NFS servers may often refresh their
concept of the user supplementary group membership at their own cadence,
it is possible for the NFS client's access cache to become stale due to
the user's group membership changing on the server after the user has
already logged in on the client.
While it is reasonable to expect that such group membership changes are
rare, and that we do not want to optimise the cache to accommodate them,
it is also not unreasonable for the user to expect that if they log out
and log back in again, that the staleness would clear up.

Reviewed-by: Benjamin Coddington &lt;bcodding@redhat.com&gt;
Tested-by: Benjamin Coddington &lt;bcodding@redhat.com&gt;
Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>nfs: Remove redundant null checks before kfree</title>
<updated>2022-10-27T19:52:10+00:00</updated>
<author>
<name>Yushan Zhou</name>
<email>katrinzhou@tencent.com</email>
</author>
<published>2022-10-18T04:07:08+00:00</published>
<link rel='alternate' type='text/html' href='https://git.toradex.cn/cgit/linux-toradex.git/commit/?id=121affdf8a940555ceef6ab10a709030e52a4f91'/>
<id>121affdf8a940555ceef6ab10a709030e52a4f91</id>
<content type='text'>
Fix the following coccicheck warning:
fs/nfs/dir.c:2494:2-7: WARNING:
NULL check before some freeing functions is not needed.

Signed-off-by: Yushan Zhou &lt;katrinzhou@tencent.com&gt;
Signed-off-by: Anna Schumaker &lt;Anna.Schumaker@Netapp.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Fix the following coccicheck warning:
fs/nfs/dir.c:2494:2-7: WARNING:
NULL check before some freeing functions is not needed.

Signed-off-by: Yushan Zhou &lt;katrinzhou@tencent.com&gt;
Signed-off-by: Anna Schumaker &lt;Anna.Schumaker@Netapp.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge tag 'pull-file_inode' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs</title>
<updated>2022-10-07T00:22:11+00:00</updated>
<author>
<name>Linus Torvalds</name>
<email>torvalds@linux-foundation.org</email>
</author>
<published>2022-10-07T00:22:11+00:00</published>
<link rel='alternate' type='text/html' href='https://git.toradex.cn/cgit/linux-toradex.git/commit/?id=ab296221579715fb8f36a27c374ebabe5bfb7e9e'/>
<id>ab296221579715fb8f36a27c374ebabe5bfb7e9e</id>
<content type='text'>
Pull file_inode() updates from Al Vrio:
 "whack-a-mole: cropped up open-coded file_inode() uses..."

* tag 'pull-file_inode' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
  orangefs: use -&gt;f_mapping
  _nfs42_proc_copy(): use -&gt;f_mapping instead of file_inode()-&gt;i_mapping
  dma_buf: no need to bother with file_inode()-&gt;i_mapping
  nfs_finish_open(): don't open-code file_inode()
  bprm_fill_uid(): don't open-code file_inode()
  sgx: use -&gt;f_mapping...
  exfat_iterate(): don't open-code file_inode(file)
  ibmvmc: don't open-code file_inode()
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Pull file_inode() updates from Al Vrio:
 "whack-a-mole: cropped up open-coded file_inode() uses..."

* tag 'pull-file_inode' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
  orangefs: use -&gt;f_mapping
  _nfs42_proc_copy(): use -&gt;f_mapping instead of file_inode()-&gt;i_mapping
  dma_buf: no need to bother with file_inode()-&gt;i_mapping
  nfs_finish_open(): don't open-code file_inode()
  bprm_fill_uid(): don't open-code file_inode()
  sgx: use -&gt;f_mapping...
  exfat_iterate(): don't open-code file_inode(file)
  ibmvmc: don't open-code file_inode()
</pre>
</div>
</content>
</entry>
<entry>
<title>nfs_finish_open(): don't open-code file_inode()</title>
<updated>2022-09-01T21:44:57+00:00</updated>
<author>
<name>Al Viro</name>
<email>viro@zeniv.linux.org.uk</email>
</author>
<published>2022-08-20T15:47:03+00:00</published>
<link rel='alternate' type='text/html' href='https://git.toradex.cn/cgit/linux-toradex.git/commit/?id=1f24cd31c28178415158366b676c2e18be812940'/>
<id>1f24cd31c28178415158366b676c2e18be812940</id>
<content type='text'>
Reviewed-by: Christian Brauner (Microsoft) &lt;brauner@kernel.org&gt;
Signed-off-by: Al Viro &lt;viro@zeniv.linux.org.uk&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Reviewed-by: Christian Brauner (Microsoft) &lt;brauner@kernel.org&gt;
Signed-off-by: Al Viro &lt;viro@zeniv.linux.org.uk&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>NFS: unlink/rmdir shouldn't call d_delete() twice on ENOENT</title>
<updated>2022-08-20T00:31:36+00:00</updated>
<author>
<name>NeilBrown</name>
<email>neilb@suse.de</email>
</author>
<published>2022-08-18T23:55:59+00:00</published>
<link rel='alternate' type='text/html' href='https://git.toradex.cn/cgit/linux-toradex.git/commit/?id=f16857e62bac60786104c020ad7c86e2163b2c5b'/>
<id>f16857e62bac60786104c020ad7c86e2163b2c5b</id>
<content type='text'>
nfs_unlink() calls d_delete() twice if it receives ENOENT from the
server - once in nfs_dentry_handle_enoent() from nfs_safe_remove and
once in nfs_dentry_remove_handle_error().

nfs_rmddir() also calls it twice - the nfs_dentry_handle_enoent() call
is direct and inside a region locked with -&gt;rmdir_sem

It is safe to call d_delete() twice if the refcount &gt; 1 as the dentry is
simply unhashed.
If the refcount is 1, the first call sets d_inode to NULL and the second
call crashes.

This patch guards the d_delete() call from nfs_dentry_handle_enoent()
leaving the one under -&gt;remdir_sem in case that is important.

In mainline it would be safe to remove the d_delete() call.  However in
older kernels to which this might be backported, that would change the
behaviour of nfs_unlink().  nfs_unlink() used to unhash the dentry which
resulted in nfs_dentry_handle_enoent() not calling d_delete().  So in
older kernels we need the d_delete() in nfs_dentry_remove_handle_error()
when called from nfs_unlink() but not when called from nfs_rmdir().

To make the code work correctly for old and new kernels, and from both
nfs_unlink() and nfs_rmdir(), we protect the d_delete() call with
simple_positive().  This ensures it is never called in a circumstance
where it could crash.

Fixes: 3c59366c207e ("NFS: don't unhash dentry during unlink/rename")
Fixes: 9019fb391de0 ("NFS: Label the dentry with a verifier in nfs_rmdir() and nfs_unlink()")
Signed-off-by: NeilBrown &lt;neilb@suse.de&gt;
Tested-by: Olga Kornievskaia &lt;aglo@umich.edu&gt;
Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
nfs_unlink() calls d_delete() twice if it receives ENOENT from the
server - once in nfs_dentry_handle_enoent() from nfs_safe_remove and
once in nfs_dentry_remove_handle_error().

nfs_rmddir() also calls it twice - the nfs_dentry_handle_enoent() call
is direct and inside a region locked with -&gt;rmdir_sem

It is safe to call d_delete() twice if the refcount &gt; 1 as the dentry is
simply unhashed.
If the refcount is 1, the first call sets d_inode to NULL and the second
call crashes.

This patch guards the d_delete() call from nfs_dentry_handle_enoent()
leaving the one under -&gt;remdir_sem in case that is important.

In mainline it would be safe to remove the d_delete() call.  However in
older kernels to which this might be backported, that would change the
behaviour of nfs_unlink().  nfs_unlink() used to unhash the dentry which
resulted in nfs_dentry_handle_enoent() not calling d_delete().  So in
older kernels we need the d_delete() in nfs_dentry_remove_handle_error()
when called from nfs_unlink() but not when called from nfs_rmdir().

To make the code work correctly for old and new kernels, and from both
nfs_unlink() and nfs_rmdir(), we protect the d_delete() call with
simple_positive().  This ensures it is never called in a circumstance
where it could crash.

Fixes: 3c59366c207e ("NFS: don't unhash dentry during unlink/rename")
Fixes: 9019fb391de0 ("NFS: Label the dentry with a verifier in nfs_rmdir() and nfs_unlink()")
Signed-off-by: NeilBrown &lt;neilb@suse.de&gt;
Tested-by: Olga Kornievskaia &lt;aglo@umich.edu&gt;
Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>NFS: Fix missing unlock in nfs_unlink()</title>
<updated>2022-08-13T17:02:13+00:00</updated>
<author>
<name>Sun Ke</name>
<email>sunke32@huawei.com</email>
</author>
<published>2022-08-12T01:14:40+00:00</published>
<link rel='alternate' type='text/html' href='https://git.toradex.cn/cgit/linux-toradex.git/commit/?id=2067231a9e2cbbcae0a4aca6ac36ff2dd6a7b701'/>
<id>2067231a9e2cbbcae0a4aca6ac36ff2dd6a7b701</id>
<content type='text'>
Add the missing unlock before goto.

Fixes: 3c59366c207e ("NFS: don't unhash dentry during unlink/rename")
Signed-off-by: Sun Ke &lt;sunke32@huawei.com&gt;
Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Add the missing unlock before goto.

Fixes: 3c59366c207e ("NFS: don't unhash dentry during unlink/rename")
Signed-off-by: Sun Ke &lt;sunke32@huawei.com&gt;
Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>NFS: don't unhash dentry during unlink/rename</title>
<updated>2022-08-08T20:25:56+00:00</updated>
<author>
<name>NeilBrown</name>
<email>neilb@suse.de</email>
</author>
<published>2022-08-01T00:33:34+00:00</published>
<link rel='alternate' type='text/html' href='https://git.toradex.cn/cgit/linux-toradex.git/commit/?id=3c59366c207e4c6c6569524af606baf017a55c61'/>
<id>3c59366c207e4c6c6569524af606baf017a55c61</id>
<content type='text'>
NFS unlink() (and rename over existing target) must determine if the
file is open, and must perform a "silly rename" instead of an unlink (or
before rename) if it is.  Otherwise the client might hold a file open
which has been removed on the server.

Consequently if it determines that the file isn't open, it must block
any subsequent opens until the unlink/rename has been completed on the
server.

This is currently achieved by unhashing the dentry.  This forces any
open attempt to the slow-path for lookup which will block on i_rwsem on
the directory until the unlink/rename completes.  A future patch will
change the VFS to only get a shared lock on i_rwsem for unlink, so this
will no longer work.

Instead we introduce an explicit interlock.  A special value is stored
in dentry-&gt;d_fsdata while the unlink/rename is running and
-&gt;d_revalidate blocks while that value is present.  When -&gt;d_revalidate
unblocks, the dentry will be invalid.  This closes the race
without requiring exclusion on i_rwsem.

d_fsdata is already used in two different ways.
1/ an IS_ROOT directory dentry might have a "devname" stored in
   d_fsdata.  Such a dentry doesn't have a name and so cannot be the
   target of unlink or rename.  For safety we check if an old devname
   is still stored, and remove it if it is.
2/ a dentry with DCACHE_NFSFS_RENAMED set will have a 'struct
   nfs_unlinkdata' stored in d_fsdata.  While this is set maydelete()
   will fail, so an unlink or rename will never proceed on such
   a dentry.

Neither of these can be in effect when a dentry is the target of unlink
or rename.  So we can expect d_fsdata to be NULL, and store a special
value ((void*)1) which is given the name NFS_FSDATA_BLOCKED to indicate
that any lookup will be blocked.

The d_count() is incremented under d_lock() when a lookup finds the
dentry, so we check d_count() is low, and set NFS_FSDATA_BLOCKED under
the same lock to avoid any races.

Signed-off-by: NeilBrown &lt;neilb@suse.de&gt;
Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
NFS unlink() (and rename over existing target) must determine if the
file is open, and must perform a "silly rename" instead of an unlink (or
before rename) if it is.  Otherwise the client might hold a file open
which has been removed on the server.

Consequently if it determines that the file isn't open, it must block
any subsequent opens until the unlink/rename has been completed on the
server.

This is currently achieved by unhashing the dentry.  This forces any
open attempt to the slow-path for lookup which will block on i_rwsem on
the directory until the unlink/rename completes.  A future patch will
change the VFS to only get a shared lock on i_rwsem for unlink, so this
will no longer work.

Instead we introduce an explicit interlock.  A special value is stored
in dentry-&gt;d_fsdata while the unlink/rename is running and
-&gt;d_revalidate blocks while that value is present.  When -&gt;d_revalidate
unblocks, the dentry will be invalid.  This closes the race
without requiring exclusion on i_rwsem.

d_fsdata is already used in two different ways.
1/ an IS_ROOT directory dentry might have a "devname" stored in
   d_fsdata.  Such a dentry doesn't have a name and so cannot be the
   target of unlink or rename.  For safety we check if an old devname
   is still stored, and remove it if it is.
2/ a dentry with DCACHE_NFSFS_RENAMED set will have a 'struct
   nfs_unlinkdata' stored in d_fsdata.  While this is set maydelete()
   will fail, so an unlink or rename will never proceed on such
   a dentry.

Neither of these can be in effect when a dentry is the target of unlink
or rename.  So we can expect d_fsdata to be NULL, and store a special
value ((void*)1) which is given the name NFS_FSDATA_BLOCKED to indicate
that any lookup will be blocked.

The d_count() is incremented under d_lock() when a lookup finds the
dentry, so we check d_count() is low, and set NFS_FSDATA_BLOCKED under
the same lock to avoid any races.

Signed-off-by: NeilBrown &lt;neilb@suse.de&gt;
Signed-off-by: Trond Myklebust &lt;trond.myklebust@hammerspace.com&gt;
</pre>
</div>
</content>
</entry>
</feed>
