Messages in this thread | | | From | (Eric W. Biederman) | Date | Wed, 09 Dec 2020 16:04:44 -0600 | Subject | Re: [PATCH] files: rcu free files_struct |
| |
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Wed, Dec 9, 2020 at 10:05 AM Eric W. Biederman <ebiederm@xmission.com> wrote: >> >> - struct file * file = xchg(&fdt->fd[i], NULL); >> + struct file * file = fdt->fd[i]; >> if (file) { >> + rcu_assign_pointer(fdt->fd[i], NULL); > > This makes me nervous. Why did we use to do that xchg() there? That > has atomicity guarantees that now are gone. > > Now, this whole thing should be called for just the last ref of the fd > table, so presumably that atomicity was never needed in the first > place. But the fact that we did that very expensive xchg() then makes > me go "there's some reason for it". > > Is this xchg() just bogus historical leftover? It kind of looks that > way. But maybe that change should be done separately?
Removing the xchg in a separate patch seems reasonable. Just to make the review easier.
I traced the xchg back to 7cf4dc3c8dbf ("move files_struct-related bits from kernel/exit.c to fs/file.c") when put_files_struct was introduced. The xchg did not exist before that change.
There were many other xchgs in the code back then so I suspect was left over from some way an earlier version of the change worked and simply was not removed when the patch was updated.
Eric
| |