On Tue, May 30, 2023 at 3:41 PM Luis Chamberlain <mcgrof@xxxxxxxxxx> wrote:
>
> OK thanks! So just to confirm, it seems fine to return the same error
> code if duplicates wait, or do you prefer for some reason for there to
> be an exception and return -EEXIST if the module did succeed in the
> duplicate case?
I think either should be fine, since either was possible before.
By definition, these are module loads being done in parallel, and so
any of them "could" have been the first, and returned success before.
And by extension, any of them could have been not first, and returned
-EEXIST if somebody else loaded the same module first.
So that "somebody else did a load" code:
if (idempotent(&idem, file_inode(f))) {
wait_for_completion(&idem.complete);
return idem.ret;
}
could certainly have made the return value be something like
return idem.ret ? : -EEXIST;
instead of that "return idem.ret".
But it does seem simpler - and more in line with the conceptual
"loading the same module is an idempotent operation" of the patch -
to just always return the success value to all of them.
After all, they all did in some sense succeed to get that module
loaded, even if it was a communal effort, and some threads did more
than others...
As mentioned, I don't think it can matter either way, since any of the
callers might as well have been the successful one, and they would
basically have to act the same way regardless (ie "somebody else
succeeded" and "you succeeded" are basically equivalent return
values). If the module was a prerequisite for another module being
loaded, either -EEXIST or 0 _is_ a success case.
Linus
