* [PATCH] nfs: block notification on fs with its own ->lock
@ 2021-12-16 17:20 J. Bruce Fields
2021-12-17 6:41 ` Vasily Averin
0 siblings, 1 reply; 7+ messages in thread
From: J. Bruce Fields @ 2021-12-16 17:20 UTC (permalink / raw)
To: Chuck Lever
Cc: linux-nfs, Vasily Averin, Jeff Layton, Denis V. Lunev,
Cyrill Gorcunov, Konstantin Khorenko
From: "J. Bruce Fields" <bfields@redhat.com>
NFSv4.1 supports an optional lock notification feature which notifies
the client when a lock comes available. (Normally NFSv4 clients just
poll for locks if necessary.) To make that work, we need to request a
blocking lock from the filesystem.
We turned that off for NFS in f657f8eef3ff "nfs: don't atempt blocking
locks on nfs reexports" because it actually blocks the nfsd thread while
waiting for the lock.
Thanks to Vasily Averin for pointing out that NFS isn't the only
filesystem with that problem.
Any filesystem that leaves ->lock NULL will use posix_lock_file(), which
does the right thing. Simplest is just to assume that any filesystem
that defines its own ->lock is not safe to request a blocking lock from.
So, this patch mostly reverts f657f8eef3ff and b840be2f00c0, and instead
uses a check of ->lock (Vasily's suggestion) to decide whether to
support blocking lock notifications on a given filesystem. Also add a
little documentation.
Perhaps someday we could add back an export flag later to allow
filesystems with "good" ->lock methods to support blocking lock
notifications.
Reported-by: Vasily Averin <vvs@virtuozzo.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/lockd/svclock.c | 2 +-
fs/nfs/export.c | 2 +-
fs/nfsd/nfs4state.c | 18 ++++++++++++------
include/linux/exportfs.h | 2 --
include/linux/lockd/lockd.h | 9 +++++++--
5 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index e9b85d8fd5fe..98e2f9b32e21 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -484,7 +484,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
(long long)lock->fl.fl_end,
wait);
- if (inode->i_sb->s_export_op->flags & EXPORT_OP_SYNC_LOCKS) {
+ if (nlmsvc_file_file(file)->f_op->lock) {
async_block = wait;
wait = 0;
}
diff --git a/fs/nfs/export.c b/fs/nfs/export.c
index 171c424cb6d5..01596f2d0a1e 100644
--- a/fs/nfs/export.c
+++ b/fs/nfs/export.c
@@ -158,5 +158,5 @@ const struct export_operations nfs_export_ops = {
.fetch_iversion = nfs_fetch_iversion,
.flags = EXPORT_OP_NOWCC|EXPORT_OP_NOSUBTREECHK|
EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS|
- EXPORT_OP_NOATOMIC_ATTR|EXPORT_OP_SYNC_LOCKS,
+ EXPORT_OP_NOATOMIC_ATTR,
};
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1956d377d1a6..3317493d2750 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6836,7 +6836,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfsd4_blocked_lock *nbl = NULL;
struct file_lock *file_lock = NULL;
struct file_lock *conflock = NULL;
- struct super_block *sb;
__be32 status = 0;
int lkflg;
int err;
@@ -6858,7 +6857,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
dprintk("NFSD: nfsd4_lock: permission denied!\n");
return status;
}
- sb = cstate->current_fh.fh_dentry->d_sb;
if (lock->lk_is_new) {
if (nfsd4_has_session(cstate))
@@ -6910,8 +6908,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
fp = lock_stp->st_stid.sc_file;
switch (lock->lk_type) {
case NFS4_READW_LT:
- if (nfsd4_has_session(cstate) &&
- !(sb->s_export_op->flags & EXPORT_OP_SYNC_LOCKS))
+ if (nfsd4_has_session(cstate))
fl_flags |= FL_SLEEP;
fallthrough;
case NFS4_READ_LT:
@@ -6923,8 +6920,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
fl_type = F_RDLCK;
break;
case NFS4_WRITEW_LT:
- if (nfsd4_has_session(cstate) &&
- !(sb->s_export_op->flags & EXPORT_OP_SYNC_LOCKS))
+ if (nfsd4_has_session(cstate))
fl_flags |= FL_SLEEP;
fallthrough;
case NFS4_WRITE_LT:
@@ -6940,6 +6936,16 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
goto out;
}
+ /*
+ * Most filesystems with their own ->lock operations will block
+ * the nfsd thread waiting to acquire the lock. That leads to
+ * deadlocks (we don't want every nfsd thread tied up waiting
+ * for file locks), so don't attempt blocking lock notifications
+ * on those filesystems:
+ */
+ if (nf->nf_file->f_op->lock)
+ fl_flags &= ~FL_SLEEP;
+
if (!nf) {
status = nfserr_openmode;
goto out;
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 3260fe714846..fe848901fcc3 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -221,8 +221,6 @@ struct export_operations {
#define EXPORT_OP_NOATOMIC_ATTR (0x10) /* Filesystem cannot supply
atomic attribute updates
*/
-#define EXPORT_OP_SYNC_LOCKS (0x20) /* Filesystem can't do
- asychronous blocking locks */
unsigned long flags;
};
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index c4ae6506b8b3..fcef192e5e45 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -303,10 +303,15 @@ void nlmsvc_invalidate_all(void);
int nlmsvc_unlock_all_by_sb(struct super_block *sb);
int nlmsvc_unlock_all_by_ip(struct sockaddr *server_addr);
+static inline struct file *nlmsvc_file_file(struct nlm_file *file)
+{
+ return file->f_file[O_RDONLY] ?
+ file->f_file[O_RDONLY] : file->f_file[O_WRONLY];
+}
+
static inline struct inode *nlmsvc_file_inode(struct nlm_file *file)
{
- return locks_inode(file->f_file[O_RDONLY] ?
- file->f_file[O_RDONLY] : file->f_file[O_WRONLY]);
+ return locks_inode(nlmsvc_file_file(file));
}
static inline int __nlm_privileged_request4(const struct sockaddr *sap)
--
2.33.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] nfs: block notification on fs with its own ->lock
2021-12-16 17:20 [PATCH] nfs: block notification on fs with its own ->lock J. Bruce Fields
@ 2021-12-17 6:41 ` Vasily Averin
2021-12-17 16:11 ` Chuck Lever III
0 siblings, 1 reply; 7+ messages in thread
From: Vasily Averin @ 2021-12-17 6:41 UTC (permalink / raw)
To: J. Bruce Fields, Chuck Lever
Cc: linux-nfs, Jeff Layton, Denis V. Lunev, Cyrill Gorcunov,
Konstantin Khorenko
On 16.12.2021 20:20, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
>
> NFSv4.1 supports an optional lock notification feature which notifies
> the client when a lock comes available. (Normally NFSv4 clients just
> poll for locks if necessary.) To make that work, we need to request a
> blocking lock from the filesystem.
>
> We turned that off for NFS in f657f8eef3ff "nfs: don't atempt blocking
> locks on nfs reexports" because it actually blocks the nfsd thread while
> waiting for the lock.
>
> Thanks to Vasily Averin for pointing out that NFS isn't the only
> filesystem with that problem.
>
> Any filesystem that leaves ->lock NULL will use posix_lock_file(), which
> does the right thing. Simplest is just to assume that any filesystem
> that defines its own ->lock is not safe to request a blocking lock from.
>
> So, this patch mostly reverts f657f8eef3ff and b840be2f00c0, and instead
> uses a check of ->lock (Vasily's suggestion) to decide whether to
> support blocking lock notifications on a given filesystem. Also add a
> little documentation.
>
> Perhaps someday we could add back an export flag later to allow
> filesystems with "good" ->lock methods to support blocking lock
> notifications.
>
> Reported-by: Vasily Averin <vvs@virtuozzo.com>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Reviewed-by: Vasily Averin <vvs@virtuozzo.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nfs: block notification on fs with its own ->lock
2021-12-17 6:41 ` Vasily Averin
@ 2021-12-17 16:11 ` Chuck Lever III
2021-12-21 11:31 ` Pratyush Yadav
0 siblings, 1 reply; 7+ messages in thread
From: Chuck Lever III @ 2021-12-17 16:11 UTC (permalink / raw)
To: Vasily Averin, Bruce Fields
Cc: Linux NFS Mailing List, Jeff Layton, Denis V. Lunev,
Cyrill Gorcunov, Konstantin Khorenko
> On Dec 17, 2021, at 1:41 AM, Vasily Averin <vvs@virtuozzo.com> wrote:
>
> On 16.12.2021 20:20, J. Bruce Fields wrote:
>> From: "J. Bruce Fields" <bfields@redhat.com>
>>
>> NFSv4.1 supports an optional lock notification feature which notifies
>> the client when a lock comes available. (Normally NFSv4 clients just
>> poll for locks if necessary.) To make that work, we need to request a
>> blocking lock from the filesystem.
>>
>> We turned that off for NFS in f657f8eef3ff "nfs: don't atempt blocking
>> locks on nfs reexports" because it actually blocks the nfsd thread while
>> waiting for the lock.
>>
>> Thanks to Vasily Averin for pointing out that NFS isn't the only
>> filesystem with that problem.
>>
>> Any filesystem that leaves ->lock NULL will use posix_lock_file(), which
>> does the right thing. Simplest is just to assume that any filesystem
>> that defines its own ->lock is not safe to request a blocking lock from.
>>
>> So, this patch mostly reverts f657f8eef3ff and b840be2f00c0, and instead
>> uses a check of ->lock (Vasily's suggestion) to decide whether to
>> support blocking lock notifications on a given filesystem. Also add a
>> little documentation.
>>
>> Perhaps someday we could add back an export flag later to allow
>> filesystems with "good" ->lock methods to support blocking lock
>> notifications.
>>
>> Reported-by: Vasily Averin <vvs@virtuozzo.com>
>> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>
> Reviewed-by: Vasily Averin <vvs@virtuozzo.com>
I've applied this with Vasily's R-b to for-next at
git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
I also cleaned up some checkpatch nits in the patch description.
It might be good for subsequent work in this area to be based
on the for-next branch so we can track what is done and what
is left to do.
--
Chuck Lever
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nfs: block notification on fs with its own ->lock
2021-12-17 16:11 ` Chuck Lever III
@ 2021-12-21 11:31 ` Pratyush Yadav
2021-12-21 15:13 ` Chuck Lever III
0 siblings, 1 reply; 7+ messages in thread
From: Pratyush Yadav @ 2021-12-21 11:31 UTC (permalink / raw)
To: Chuck Lever III
Cc: Vasily Averin, Bruce Fields, Linux NFS Mailing List, Jeff Layton,
Denis V. Lunev, Cyrill Gorcunov, Konstantin Khorenko
Hi,
On 17/12/21 04:11PM, Chuck Lever III wrote:
>
> > On Dec 17, 2021, at 1:41 AM, Vasily Averin <vvs@virtuozzo.com> wrote:
> >
> > On 16.12.2021 20:20, J. Bruce Fields wrote:
> >> From: "J. Bruce Fields" <bfields@redhat.com>
> >>
> >> NFSv4.1 supports an optional lock notification feature which notifies
> >> the client when a lock comes available. (Normally NFSv4 clients just
> >> poll for locks if necessary.) To make that work, we need to request a
> >> blocking lock from the filesystem.
> >>
> >> We turned that off for NFS in f657f8eef3ff "nfs: don't atempt blocking
> >> locks on nfs reexports" because it actually blocks the nfsd thread while
> >> waiting for the lock.
> >>
> >> Thanks to Vasily Averin for pointing out that NFS isn't the only
> >> filesystem with that problem.
> >>
> >> Any filesystem that leaves ->lock NULL will use posix_lock_file(), which
> >> does the right thing. Simplest is just to assume that any filesystem
> >> that defines its own ->lock is not safe to request a blocking lock from.
> >>
> >> So, this patch mostly reverts f657f8eef3ff and b840be2f00c0, and instead
> >> uses a check of ->lock (Vasily's suggestion) to decide whether to
> >> support blocking lock notifications on a given filesystem. Also add a
> >> little documentation.
> >>
> >> Perhaps someday we could add back an export flag later to allow
> >> filesystems with "good" ->lock methods to support blocking lock
> >> notifications.
> >>
> >> Reported-by: Vasily Averin <vvs@virtuozzo.com>
> >> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> >
> > Reviewed-by: Vasily Averin <vvs@virtuozzo.com>
>
> I've applied this with Vasily's R-b to for-next at
>
> git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
>
> I also cleaned up some checkpatch nits in the patch description.
>
> It might be good for subsequent work in this area to be based
> on the for-next branch so we can track what is done and what
> is left to do.
This patch breaks LLVM build on linux-next for me:
fs/lockd/svclock.c:474:17: error: unused variable 'inode' [-Werror,-Wunused-variable]
struct inode *inode = nlmsvc_file_inode(file);
This is because now the only user of inode is the dprintk() call, and
this is probably a noop when debug is disabled. I think you should wrap
the declaration of inode under the same debug symbol used to select
dprintk(). My LSP (ccls) is getting confused and can't point out where
exactly this macro is declared, so I am not sure which symbol controls
it (CONFIG_DEBUG? CONFIG_NFS_DEBUG?).
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nfs: block notification on fs with its own ->lock
2021-12-21 11:31 ` Pratyush Yadav
@ 2021-12-21 15:13 ` Chuck Lever III
2021-12-21 19:30 ` Pratyush Yadav
0 siblings, 1 reply; 7+ messages in thread
From: Chuck Lever III @ 2021-12-21 15:13 UTC (permalink / raw)
To: Pratyush Yadav
Cc: Vasily Averin, Bruce Fields, Linux NFS Mailing List, Jeff Layton,
Denis V. Lunev, Cyrill Gorcunov, Konstantin Khorenko
> On Dec 21, 2021, at 6:31 AM, Pratyush Yadav <p.yadav@ti.com> wrote:
>
> Hi,
>
> On 17/12/21 04:11PM, Chuck Lever III wrote:
>>
>>> On Dec 17, 2021, at 1:41 AM, Vasily Averin <vvs@virtuozzo.com> wrote:
>>>
>>> On 16.12.2021 20:20, J. Bruce Fields wrote:
>>>> From: "J. Bruce Fields" <bfields@redhat.com>
>>>>
>>>> NFSv4.1 supports an optional lock notification feature which notifies
>>>> the client when a lock comes available. (Normally NFSv4 clients just
>>>> poll for locks if necessary.) To make that work, we need to request a
>>>> blocking lock from the filesystem.
>>>>
>>>> We turned that off for NFS in f657f8eef3ff "nfs: don't atempt blocking
>>>> locks on nfs reexports" because it actually blocks the nfsd thread while
>>>> waiting for the lock.
>>>>
>>>> Thanks to Vasily Averin for pointing out that NFS isn't the only
>>>> filesystem with that problem.
>>>>
>>>> Any filesystem that leaves ->lock NULL will use posix_lock_file(), which
>>>> does the right thing. Simplest is just to assume that any filesystem
>>>> that defines its own ->lock is not safe to request a blocking lock from.
>>>>
>>>> So, this patch mostly reverts f657f8eef3ff and b840be2f00c0, and instead
>>>> uses a check of ->lock (Vasily's suggestion) to decide whether to
>>>> support blocking lock notifications on a given filesystem. Also add a
>>>> little documentation.
>>>>
>>>> Perhaps someday we could add back an export flag later to allow
>>>> filesystems with "good" ->lock methods to support blocking lock
>>>> notifications.
>>>>
>>>> Reported-by: Vasily Averin <vvs@virtuozzo.com>
>>>> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>>>
>>> Reviewed-by: Vasily Averin <vvs@virtuozzo.com>
>>
>> I've applied this with Vasily's R-b to for-next at
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
>>
>> I also cleaned up some checkpatch nits in the patch description.
>>
>> It might be good for subsequent work in this area to be based
>> on the for-next branch so we can track what is done and what
>> is left to do.
>
> This patch breaks LLVM build on linux-next for me:
>
> fs/lockd/svclock.c:474:17: error: unused variable 'inode' [-Werror,-Wunused-variable]
> struct inode *inode = nlmsvc_file_inode(file);
>
> This is because now the only user of inode is the dprintk() call, and
> this is probably a noop when debug is disabled. I think you should wrap
> the declaration of inode under the same debug symbol used to select
> dprintk(). My LSP (ccls) is getting confused and can't point out where
> exactly this macro is declared, so I am not sure which symbol controls
> it (CONFIG_DEBUG? CONFIG_NFS_DEBUG?).
I updated this patch in my for-next tree yesterday to take care of
the issue. The change has been merged into today's linux-next.
--
Chuck Lever
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nfs: block notification on fs with its own ->lock
2021-12-21 15:13 ` Chuck Lever III
@ 2021-12-21 19:30 ` Pratyush Yadav
0 siblings, 0 replies; 7+ messages in thread
From: Pratyush Yadav @ 2021-12-21 19:30 UTC (permalink / raw)
To: Chuck Lever III
Cc: Vasily Averin, Bruce Fields, Linux NFS Mailing List, Jeff Layton,
Denis V. Lunev, Cyrill Gorcunov, Konstantin Khorenko
On 21/12/21 03:13PM, Chuck Lever III wrote:
>
>
> > On Dec 21, 2021, at 6:31 AM, Pratyush Yadav <p.yadav@ti.com> wrote:
> >
> > Hi,
> >
> > On 17/12/21 04:11PM, Chuck Lever III wrote:
> >>
> >>> On Dec 17, 2021, at 1:41 AM, Vasily Averin <vvs@virtuozzo.com> wrote:
> >>>
> >>> On 16.12.2021 20:20, J. Bruce Fields wrote:
> >>>> From: "J. Bruce Fields" <bfields@redhat.com>
> >>>>
> >>>> NFSv4.1 supports an optional lock notification feature which notifies
> >>>> the client when a lock comes available. (Normally NFSv4 clients just
> >>>> poll for locks if necessary.) To make that work, we need to request a
> >>>> blocking lock from the filesystem.
> >>>>
> >>>> We turned that off for NFS in f657f8eef3ff "nfs: don't atempt blocking
> >>>> locks on nfs reexports" because it actually blocks the nfsd thread while
> >>>> waiting for the lock.
> >>>>
> >>>> Thanks to Vasily Averin for pointing out that NFS isn't the only
> >>>> filesystem with that problem.
> >>>>
> >>>> Any filesystem that leaves ->lock NULL will use posix_lock_file(), which
> >>>> does the right thing. Simplest is just to assume that any filesystem
> >>>> that defines its own ->lock is not safe to request a blocking lock from.
> >>>>
> >>>> So, this patch mostly reverts f657f8eef3ff and b840be2f00c0, and instead
> >>>> uses a check of ->lock (Vasily's suggestion) to decide whether to
> >>>> support blocking lock notifications on a given filesystem. Also add a
> >>>> little documentation.
> >>>>
> >>>> Perhaps someday we could add back an export flag later to allow
> >>>> filesystems with "good" ->lock methods to support blocking lock
> >>>> notifications.
> >>>>
> >>>> Reported-by: Vasily Averin <vvs@virtuozzo.com>
> >>>> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> >>>
> >>> Reviewed-by: Vasily Averin <vvs@virtuozzo.com>
> >>
> >> I've applied this with Vasily's R-b to for-next at
> >>
> >> git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
> >>
> >> I also cleaned up some checkpatch nits in the patch description.
> >>
> >> It might be good for subsequent work in this area to be based
> >> on the for-next branch so we can track what is done and what
> >> is left to do.
> >
> > This patch breaks LLVM build on linux-next for me:
> >
> > fs/lockd/svclock.c:474:17: error: unused variable 'inode' [-Werror,-Wunused-variable]
> > struct inode *inode = nlmsvc_file_inode(file);
> >
> > This is because now the only user of inode is the dprintk() call, and
> > this is probably a noop when debug is disabled. I think you should wrap
> > the declaration of inode under the same debug symbol used to select
> > dprintk(). My LSP (ccls) is getting confused and can't point out where
> > exactly this macro is declared, so I am not sure which symbol controls
> > it (CONFIG_DEBUG? CONFIG_NFS_DEBUG?).
>
> I updated this patch in my for-next tree yesterday to take care of
> the issue. The change has been merged into today's linux-next.
Thanks. I updated to today's linux-next and I no longer see this error.
The build still fails though, this time on some firmware driver. I'll go
bother their maintainers about it now ;-)
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nfs: block notification on fs with its own ->lock
@ 2021-12-18 23:32 kernel test robot
0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-12-18 23:32 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 22566 bytes --]
CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20211216172013.GA13418@fieldses.org>
References: <20211216172013.GA13418@fieldses.org>
TO: bfields(a)fieldses.org
Hi,
I love your patch! Perhaps something to improve:
[auto build test WARNING on trondmy-nfs/linux-next]
[also build test WARNING on linus/master v5.16-rc5 next-20211217]
[cannot apply to cel-2.6/for-next jlayton/linux-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/bfields-fieldses-org-J-Bruce-Fields/nfs-block-notification-on-fs-with-its-own-lock/20211217-012235
base: git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago
config: x86_64-randconfig-c007-20211216 (https://download.01.org/0day-ci/archive/20211219/202112190732.2tW2ZDsC-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 9043c3d65b11b442226015acfbf8167684586cfa)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/61c7ebf40fcfd8350ff7d4b2101c18ea234fbcae
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review bfields-fieldses-org-J-Bruce-Fields/nfs-block-notification-on-fs-with-its-own-lock/20211217-012235
git checkout 61c7ebf40fcfd8350ff7d4b2101c18ea234fbcae
# save the config file to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 clang-analyzer
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
clang-analyzer warnings: (new ones prefixed by >>)
^~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:304:6: note: 'block' is not equal to NULL
if (block != NULL)
^~~~~
fs/lockd/svclock.c:304:2: note: Taking true branch
if (block != NULL)
^
fs/lockd/svclock.c:305:3: note: Calling 'kref_put_mutex'
kref_put_mutex(&block->b_count, nlmsvc_free_block, &block->b_file->f_mutex);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/kref.h:75:6: note: Assuming the condition is true
if (refcount_dec_and_mutex_lock(&kref->refcount, lock)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/kref.h:75:2: note: Taking true branch
if (refcount_dec_and_mutex_lock(&kref->refcount, lock)) {
^
include/linux/kref.h:76:3: note: Calling 'nlmsvc_free_block'
release(kref);
^~~~~~~~~~~~~
fs/lockd/svclock.c:287:28: note: Left side of '&&' is false
struct nlm_block *block = container_of(kref, struct nlm_block, b_count);
^
include/linux/kernel.h:495:61: note: expanded from macro 'container_of'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^
fs/lockd/svclock.c:287:28: note: Taking false branch
struct nlm_block *block = container_of(kref, struct nlm_block, b_count);
^
include/linux/kernel.h:495:2: note: expanded from macro 'container_of'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^
include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^
include/linux/compiler_types.h:322:2: note: expanded from macro 'compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
^
include/linux/compiler_types.h:310:2: note: expanded from macro '_compiletime_assert'
__compiletime_assert(condition, msg, prefix, suffix)
^
include/linux/compiler_types.h:302:3: note: expanded from macro '__compiletime_assert'
if (!(condition)) \
^
fs/lockd/svclock.c:287:28: note: Loop condition is false. Exiting loop
struct nlm_block *block = container_of(kref, struct nlm_block, b_count);
^
include/linux/kernel.h:495:2: note: expanded from macro 'container_of'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^
include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^
include/linux/compiler_types.h:322:2: note: expanded from macro 'compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
^
include/linux/compiler_types.h:310:2: note: expanded from macro '_compiletime_assert'
__compiletime_assert(condition, msg, prefix, suffix)
^
include/linux/compiler_types.h:300:2: note: expanded from macro '__compiletime_assert'
do { \
^
fs/lockd/svclock.c:290:2: note: Loop condition is false. Exiting loop
dprintk("lockd: freeing block %p...\n", block);
^
include/linux/sunrpc/debug.h:25:2: note: expanded from macro 'dprintk'
dfprintk(FACILITY, fmt, ##__VA_ARGS__)
^
include/linux/sunrpc/debug.h:70:34: note: expanded from macro 'dfprintk'
# define dfprintk(fac, fmt, ...) do {} while (0)
^
fs/lockd/svclock.c:299:2: note: Memory is released
kfree(block);
^~~~~~~~~~~~
include/linux/kref.h:76:3: note: Returning; memory was released
release(kref);
^~~~~~~~~~~~~
fs/lockd/svclock.c:305:3: note: Returning; memory was released
kref_put_mutex(&block->b_count, nlmsvc_free_block, &block->b_file->f_mutex);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:138:3: note: Returning; memory was released
nlmsvc_release_block(block);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:281:2: note: Returning; memory was released
nlmsvc_remove_block(block);
^~~~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:974:3: note: Returning; memory was released
nlmsvc_unlink_block(block);
^~~~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:976:2: note: Calling 'nlmsvc_release_block'
nlmsvc_release_block(block);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:304:6: note: 'block' is not equal to NULL
if (block != NULL)
^~~~~
fs/lockd/svclock.c:304:2: note: Taking true branch
if (block != NULL)
^
fs/lockd/svclock.c:305:55: note: Use of memory after it is freed
kref_put_mutex(&block->b_count, nlmsvc_free_block, &block->b_file->f_mutex);
^~~~~~~~~~~~~
>> fs/lockd/svclock.c:474:17: warning: Value stored to 'inode' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
struct inode *inode = nlmsvc_file_inode(file);
^~~~~ ~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:474:17: note: Value stored to 'inode' during its initialization is never read
struct inode *inode = nlmsvc_file_inode(file);
^~~~~ ~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:575:2: warning: Use of memory after it is freed [clang-analyzer-unix.Malloc]
nlmsvc_release_block(block);
^ ~~~~~
fs/lockd/svclock.c:480:2: note: Loop condition is false. Exiting loop
dprintk("lockd: nlmsvc_lock(%s/%ld, ty=%d, pi=%d, %Ld-%Ld, bl=%d)\n",
^
include/linux/sunrpc/debug.h:25:2: note: expanded from macro 'dprintk'
dfprintk(FACILITY, fmt, ##__VA_ARGS__)
^
include/linux/sunrpc/debug.h:70:34: note: expanded from macro 'dfprintk'
# define dfprintk(fac, fmt, ...) do {} while (0)
^
fs/lockd/svclock.c:487:6: note: Assuming field 'lock' is null
if (nlmsvc_file_file(file)->f_op->lock) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:487:2: note: Taking false branch
if (nlmsvc_file_file(file)->f_op->lock) {
^
fs/lockd/svclock.c:498:6: note: Assuming 'block' is not equal to NULL
if (block == NULL) {
^~~~~~~~~~~~~
fs/lockd/svclock.c:498:2: note: Taking false branch
if (block == NULL) {
^
fs/lockd/svclock.c:507:6: note: Assuming the condition is true
if (block->b_flags & B_QUEUED) {
^~~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:507:2: note: Taking true branch
if (block->b_flags & B_QUEUED) {
^
fs/lockd/svclock.c:508:3: note: Loop condition is false. Exiting loop
dprintk("lockd: nlmsvc_lock deferred block %p flags %d\n",
^
include/linux/sunrpc/debug.h:25:2: note: expanded from macro 'dprintk'
dfprintk(FACILITY, fmt, ##__VA_ARGS__)
^
include/linux/sunrpc/debug.h:70:34: note: expanded from macro 'dfprintk'
# define dfprintk(fac, fmt, ...) do {} while (0)
^
fs/lockd/svclock.c:510:7: note: Assuming field 'b_granted' is not equal to 0
if (block->b_granted) {
^~~~~~~~~~~~~~~~
fs/lockd/svclock.c:510:3: note: Taking true branch
if (block->b_granted) {
^
fs/lockd/svclock.c:511:4: note: Calling 'nlmsvc_unlink_block'
nlmsvc_unlink_block(block);
^~~~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:277:2: note: Loop condition is false. Exiting loop
dprintk("lockd: unlinking block %p...\n", block);
^
include/linux/sunrpc/debug.h:25:2: note: expanded from macro 'dprintk'
dfprintk(FACILITY, fmt, ##__VA_ARGS__)
^
include/linux/sunrpc/debug.h:70:34: note: expanded from macro 'dfprintk'
# define dfprintk(fac, fmt, ...) do {} while (0)
^
fs/lockd/svclock.c:281:2: note: Calling 'nlmsvc_remove_block'
nlmsvc_remove_block(block);
^~~~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:134:6: note: Assuming the condition is true
if (!list_empty(&block->b_list)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:134:2: note: Taking true branch
if (!list_empty(&block->b_list)) {
^
fs/lockd/svclock.c:138:3: note: Calling 'nlmsvc_release_block'
nlmsvc_release_block(block);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/lockd/svclock.c:304:6: note: 'block' is not equal to NULL
if (block != NULL)
^~~~~
fs/lockd/svclock.c:304:2: note: Taking true branch
if (block != NULL)
^
fs/lockd/svclock.c:305:3: note: Calling 'kref_put_mutex'
kref_put_mutex(&block->b_count, nlmsvc_free_block, &block->b_file->f_mutex);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/kref.h:75:6: note: Assuming the condition is true
if (refcount_dec_and_mutex_lock(&kref->refcount, lock)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/kref.h:75:2: note: Taking true branch
if (refcount_dec_and_mutex_lock(&kref->refcount, lock)) {
^
include/linux/kref.h:76:3: note: Calling 'nlmsvc_free_block'
release(kref);
^~~~~~~~~~~~~
fs/lockd/svclock.c:287:28: note: Left side of '&&' is false
struct nlm_block *block = container_of(kref, struct nlm_block, b_count);
^
include/linux/kernel.h:495:61: note: expanded from macro 'container_of'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^
fs/lockd/svclock.c:287:28: note: Taking false branch
struct nlm_block *block = container_of(kref, struct nlm_block, b_count);
vim +/inode +474 fs/lockd/svclock.c
2b36f412ab6f2e Marc Eshel 2006-11-28 463
^1da177e4c3f41 Linus Torvalds 2005-04-16 464 /*
^1da177e4c3f41 Linus Torvalds 2005-04-16 465 * Attempt to establish a lock, and if it can't be granted, block it
^1da177e4c3f41 Linus Torvalds 2005-04-16 466 * if required.
^1da177e4c3f41 Linus Torvalds 2005-04-16 467 */
52921e02a4f416 Al Viro 2006-10-19 468 __be32
^1da177e4c3f41 Linus Torvalds 2005-04-16 469 nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
6cde4de8077349 Jeff Layton 2008-07-15 470 struct nlm_host *host, struct nlm_lock *lock, int wait,
b2b5028905226f J. Bruce Fields 2008-02-06 471 struct nlm_cookie *cookie, int reclaim)
^1da177e4c3f41 Linus Torvalds 2005-04-16 472 {
f812048020282f Marc Eshel 2006-12-05 473 struct nlm_block *block = NULL;
b840be2f00c0bc J. Bruce Fields 2021-08-20 @474 struct inode *inode = nlmsvc_file_inode(file);
^1da177e4c3f41 Linus Torvalds 2005-04-16 475 int error;
7f024fcd5c97dc J. Bruce Fields 2021-08-23 476 int mode;
b840be2f00c0bc J. Bruce Fields 2021-08-20 477 int async_block = 0;
52921e02a4f416 Al Viro 2006-10-19 478 __be32 ret;
^1da177e4c3f41 Linus Torvalds 2005-04-16 479
^1da177e4c3f41 Linus Torvalds 2005-04-16 480 dprintk("lockd: nlmsvc_lock(%s/%ld, ty=%d, pi=%d, %Ld-%Ld, bl=%d)\n",
b840be2f00c0bc J. Bruce Fields 2021-08-20 481 inode->i_sb->s_id, inode->i_ino,
^1da177e4c3f41 Linus Torvalds 2005-04-16 482 lock->fl.fl_type, lock->fl.fl_pid,
^1da177e4c3f41 Linus Torvalds 2005-04-16 483 (long long)lock->fl.fl_start,
^1da177e4c3f41 Linus Torvalds 2005-04-16 484 (long long)lock->fl.fl_end,
^1da177e4c3f41 Linus Torvalds 2005-04-16 485 wait);
^1da177e4c3f41 Linus Torvalds 2005-04-16 486
61c7ebf40fcfd8 J. Bruce Fields 2021-12-16 487 if (nlmsvc_file_file(file)->f_op->lock) {
b840be2f00c0bc J. Bruce Fields 2021-08-20 488 async_block = wait;
b840be2f00c0bc J. Bruce Fields 2021-08-20 489 wait = 0;
b840be2f00c0bc J. Bruce Fields 2021-08-20 490 }
b840be2f00c0bc J. Bruce Fields 2021-08-20 491
^1da177e4c3f41 Linus Torvalds 2005-04-16 492 /* Lock file against concurrent access */
89e63ef609fb00 Neil Brown 2006-10-04 493 mutex_lock(&file->f_mutex);
f812048020282f Marc Eshel 2006-12-05 494 /* Get existing block (in case client is busy-waiting)
f812048020282f Marc Eshel 2006-12-05 495 * or create new block
f812048020282f Marc Eshel 2006-12-05 496 */
d9f6eb75d49007 Trond Myklebust 2006-03-20 497 block = nlmsvc_lookup_block(file, lock);
09c7938c5640a6 Trond Myklebust 2006-03-20 498 if (block == NULL) {
560de0e65904db J. Bruce Fields 2008-07-15 499 block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
f812048020282f Marc Eshel 2006-12-05 500 ret = nlm_lck_denied_nolocks;
f812048020282f Marc Eshel 2006-12-05 501 if (block == NULL)
f812048020282f Marc Eshel 2006-12-05 502 goto out;
92737230dd3f14 Trond Myklebust 2006-03-20 503 lock = &block->b_call->a_args.lock;
f812048020282f Marc Eshel 2006-12-05 504 } else
f812048020282f Marc Eshel 2006-12-05 505 lock->fl.fl_flags &= ~FL_SLEEP;
^1da177e4c3f41 Linus Torvalds 2005-04-16 506
1a8322b2b02071 Marc Eshel 2006-11-28 507 if (block->b_flags & B_QUEUED) {
1a8322b2b02071 Marc Eshel 2006-11-28 508 dprintk("lockd: nlmsvc_lock deferred block %p flags %d\n",
1a8322b2b02071 Marc Eshel 2006-11-28 509 block, block->b_flags);
1a8322b2b02071 Marc Eshel 2006-11-28 510 if (block->b_granted) {
1a8322b2b02071 Marc Eshel 2006-11-28 511 nlmsvc_unlink_block(block);
1a8322b2b02071 Marc Eshel 2006-11-28 512 ret = nlm_granted;
1a8322b2b02071 Marc Eshel 2006-11-28 513 goto out;
1a8322b2b02071 Marc Eshel 2006-11-28 514 }
1a8322b2b02071 Marc Eshel 2006-11-28 515 if (block->b_flags & B_TIMED_OUT) {
1a8322b2b02071 Marc Eshel 2006-11-28 516 nlmsvc_unlink_block(block);
1a8322b2b02071 Marc Eshel 2006-11-28 517 ret = nlm_lck_denied;
1a8322b2b02071 Marc Eshel 2006-11-28 518 goto out;
1a8322b2b02071 Marc Eshel 2006-11-28 519 }
1a8322b2b02071 Marc Eshel 2006-11-28 520 ret = nlm_drop_reply;
1a8322b2b02071 Marc Eshel 2006-11-28 521 goto out;
1a8322b2b02071 Marc Eshel 2006-11-28 522 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 523
5ccb0066f2d561 Stanislav Kinsbursky 2012-07-25 524 if (locks_in_grace(SVC_NET(rqstp)) && !reclaim) {
b2b5028905226f J. Bruce Fields 2008-02-06 525 ret = nlm_lck_denied_grace_period;
b2b5028905226f J. Bruce Fields 2008-02-06 526 goto out;
b2b5028905226f J. Bruce Fields 2008-02-06 527 }
5ccb0066f2d561 Stanislav Kinsbursky 2012-07-25 528 if (reclaim && !locks_in_grace(SVC_NET(rqstp))) {
d22b1cff099737 J. Bruce Fields 2008-02-06 529 ret = nlm_lck_denied_grace_period;
d22b1cff099737 J. Bruce Fields 2008-02-06 530 goto out;
d22b1cff099737 J. Bruce Fields 2008-02-06 531 }
b2b5028905226f J. Bruce Fields 2008-02-06 532
1a8322b2b02071 Marc Eshel 2006-11-28 533 if (!wait)
1a8322b2b02071 Marc Eshel 2006-11-28 534 lock->fl.fl_flags &= ~FL_SLEEP;
7f024fcd5c97dc J. Bruce Fields 2021-08-23 535 mode = lock_to_openmode(&lock->fl);
7f024fcd5c97dc J. Bruce Fields 2021-08-23 536 error = vfs_lock_file(file->f_file[mode], F_SETLK, &lock->fl, NULL);
1a8322b2b02071 Marc Eshel 2006-11-28 537 lock->fl.fl_flags &= ~FL_SLEEP;
a85f193e2fb7d5 Andy Adamson 2006-03-20 538
1a8322b2b02071 Marc Eshel 2006-11-28 539 dprintk("lockd: vfs_lock_file returned %d\n", error);
09c7938c5640a6 Trond Myklebust 2006-03-20 540 switch (error) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 541 case 0:
15dadef9460ad8 Andy Adamson 2006-03-20 542 ret = nlm_granted;
15dadef9460ad8 Andy Adamson 2006-03-20 543 goto out;
09c7938c5640a6 Trond Myklebust 2006-03-20 544 case -EAGAIN:
e33d1ea60c3a17 Miklos Szeredi 2009-02-09 545 /*
e33d1ea60c3a17 Miklos Szeredi 2009-02-09 546 * If this is a blocking request for an
e33d1ea60c3a17 Miklos Szeredi 2009-02-09 547 * already pending lock request then we need
e33d1ea60c3a17 Miklos Szeredi 2009-02-09 548 * to put it back on lockd's block list
e33d1ea60c3a17 Miklos Szeredi 2009-02-09 549 */
e33d1ea60c3a17 Miklos Szeredi 2009-02-09 550 if (wait)
9d9b87c1218be7 J. Bruce Fields 2009-02-04 551 break;
b840be2f00c0bc J. Bruce Fields 2021-08-20 552 ret = async_block ? nlm_lck_blocked : nlm_lck_denied;
e33d1ea60c3a17 Miklos Szeredi 2009-02-09 553 goto out;
bde74e4bc64415 Miklos Szeredi 2008-07-25 554 case FILE_LOCK_DEFERRED:
1a8322b2b02071 Marc Eshel 2006-11-28 555 if (wait)
1a8322b2b02071 Marc Eshel 2006-11-28 556 break;
1a8322b2b02071 Marc Eshel 2006-11-28 557 /* Filesystem lock operation is in progress
1a8322b2b02071 Marc Eshel 2006-11-28 558 Add it to the queue waiting for callback */
1a8322b2b02071 Marc Eshel 2006-11-28 559 ret = nlmsvc_defer_lock_rqst(rqstp, block);
1a8322b2b02071 Marc Eshel 2006-11-28 560 goto out;
09c7938c5640a6 Trond Myklebust 2006-03-20 561 case -EDEADLK:
15dadef9460ad8 Andy Adamson 2006-03-20 562 ret = nlm_deadlock;
15dadef9460ad8 Andy Adamson 2006-03-20 563 goto out;
^1da177e4c3f41 Linus Torvalds 2005-04-16 564 default: /* includes ENOLCK */
15dadef9460ad8 Andy Adamson 2006-03-20 565 ret = nlm_lck_denied_nolocks;
15dadef9460ad8 Andy Adamson 2006-03-20 566 goto out;
^1da177e4c3f41 Linus Torvalds 2005-04-16 567 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 568
09c7938c5640a6 Trond Myklebust 2006-03-20 569 ret = nlm_lck_blocked;
^1da177e4c3f41 Linus Torvalds 2005-04-16 570
^1da177e4c3f41 Linus Torvalds 2005-04-16 571 /* Append to list of blocked */
f812048020282f Marc Eshel 2006-12-05 572 nlmsvc_insert_block(block, NLM_NEVER);
15dadef9460ad8 Andy Adamson 2006-03-20 573 out:
89e63ef609fb00 Neil Brown 2006-10-04 574 mutex_unlock(&file->f_mutex);
6849c0cab69f5d Trond Myklebust 2006-03-20 575 nlmsvc_release_block(block);
15dadef9460ad8 Andy Adamson 2006-03-20 576 dprintk("lockd: nlmsvc_lock returned %u\n", ret);
15dadef9460ad8 Andy Adamson 2006-03-20 577 return ret;
^1da177e4c3f41 Linus Torvalds 2005-04-16 578 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 579
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-12-21 19:30 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-16 17:20 [PATCH] nfs: block notification on fs with its own ->lock J. Bruce Fields
2021-12-17 6:41 ` Vasily Averin
2021-12-17 16:11 ` Chuck Lever III
2021-12-21 11:31 ` Pratyush Yadav
2021-12-21 15:13 ` Chuck Lever III
2021-12-21 19:30 ` Pratyush Yadav
2021-12-18 23:32 kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.