linux-kernel - Re: [PATCH/RFC] core/nfsd: allow kernel threads to use task

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231129075532.GE22743@redhat.com>
Date:   Wed, 29 Nov 2023 08:55:32 +0100
From:   Oleg Nesterov <oleg@...hat.com>
To:     NeilBrown <neilb@...e.de>
Cc:     Al Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>,
        Jens Axboe <axboe@...nel.dk>,
        Chuck Lever <chuck.lever@...cle.com>,
        Jeff Layton <jlayton@...nel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Juri Lelli <juri.lelli@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-nfs@...r.kernel.org
Subject: Re: [PATCH/RFC] core/nfsd: allow kernel threads to use task_work.

On 11/29, NeilBrown wrote:
>
> On Wed, 29 Nov 2023, Oleg Nesterov wrote:
> > On 11/28, NeilBrown wrote:
> > >
> > > I have evidence from a customer site of 256 nfsd threads adding files to
> > > delayed_fput_lists nearly twice as fast they are retired by a single
> > > work-queue thread running delayed_fput().  As you might imagine this
> > > does not end well (20 million files in the queue at the time a snapshot
> > > was taken for analysis).
> >
> > On a related note... Neil, Al, et al, can you look at
> >
> > 	[PATCH 1/3] fput: don't abuse task_work_add() when possible
> > 	https://lore.kernel.org/all/20150908171446.GA14589@redhat.com/
> >
>
> Would it make sense to create a separate task_struct->delayed_fput
> llist?

Sure, I too thought about this,

> fput() adds the file to this llist and if it was the first item on the
> list, it then adds the task_work.  That would probably request adding a
> callback_head to struct task_struct as well.

Even simpler, but perhaps I missed something...

We can add a "struct file *fput_xxx" into task_struct and f_fput_xxx into
the f_llist/f_rcuhead union in the struct file.

fput:

	if (task->fput_xxx) {
		file->f_fput_xxx = task->fput_xxx;
		task->fput_xxx = file;
	} else {
		task_work_add(...);
		// XXX: file->f_fput_xxx != NULL
		task->fput_xxx = file;
	}

____fput:

	struct file *file = task->fput_xxx;
	struct file *tail = container_of(work, ...);
	// see XXX in fput()
	tail->f_fput_xxx = NULL;
	current->fput_xxx = NULL;

	do {
		next = READ_ONCE(file->f_fput_xxx);
		__fput(file);
		file = next;
		
	} while (file);
	
Again, quite possibly I missed something, but something like this should work.

But I am still trying to find a simpler solution which doesn't need another
member in task_struct...

Oleg.