linux-kernel - Re: [PATCH 2/2] fs: make several inode lock operations killable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAKPOu+8H11mcMEn5gQYcJs5BhTt8J8Cypz73Vdp_tTHZRXgOKg@mail.gmail.com>
Date: Tue, 29 Apr 2025 13:28:49 +0200
From: Max Kellermann <max.kellermann@...os.com>
To: Christian Brauner <brauner@...nel.org>
Cc: viro@...iv.linux.org.uk, jack@...e.cz, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] fs: make several inode lock operations killable

On Tue, Apr 29, 2025 at 1:12 PM Christian Brauner <brauner@...nel.org> wrote:
> > --- a/fs/read_write.c
> > +++ b/fs/read_write.c
> > @@ -332,7 +332,9 @@ loff_t default_llseek(struct file *file, loff_t offset, int whence)
> >       struct inode *inode = file_inode(file);
> >       loff_t retval;
> >
> > -     inode_lock(inode);
> > +     retval = inode_lock_killable(inode);
>
> That change doesn't seem so obviously fine to me.

Why do you think so? And how is this different than the other two.

> Either way I'd like to see this split in three patches and some
> reasoning why it's safe and some justification why it's wanted...

Sure I can split this patch, but before I spend the time, I'd like us
first to agree that the patch is useful.

I wrote this while debugging lots of netfs/nfs/ceph bugs; even without
these bugs, I/O operations on netfs can take a looong time (if the
server is slow) and the inode is locked during the whole operation.
That can cause lots of other processes to go stuck, and my patch
allows these operations to be canceled. Without this, the processes
not only remain stuck until the inode is unlocked, but all stuck
processes have to finish all their I/O before anything can continue.
I'd like to be able to "kill -9" stuck processes.

A similar NFS-specific patch I wrote was merged last year:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=38a125b31504f91bf6fdd3cfc3a3e9a721e6c97a
The same patch for Ceph was never merged (but not explicitly
rejected): https://lore.kernel.org/lkml/20241206165014.165614-1-max.kellermann@ionos.com/
Prior to my work, several NFS operations were already killable.