Re: [PATCH] modules: fix livelock in add_unformed_module()

[Jessica Yu]

+++ Prarit Bhargava [28/05/19 10:30 -0400]:
> On 5/22/19 1:08 PM, Prarit Bhargava wrote:
> > On 5/13/19 10:37 AM, Barret Rhoden wrote:
> > > Hi -
> >
> > Hey Barret, my apologies for not getting back to you earlier.  I got caught up
> > in something that took me away from this issue.
> >
> > > On 5/13/19 7:23 AM, Prarit Bhargava wrote:
> > > [snip]
> > > > A module is loaded once for each cpu.
> > >
> > > Does one CPU succeed in loading the module, and the others fail with EEXIST?
> > >
> > > > My follow-up patch changes from wait_event_interruptible() to
> > > > wait_event_interruptible_timeout() so the CPUs are no longer sleeping and can
> > > > make progress on other tasks, which changes the return values from
> > > > wait_event_interruptible().
> > > >
> > > > https://marc.info/?l=linux-kernel&m=155724085927589&w=2
> > > >
> > > > I believe this also takes your concern into account?
> > >
> > > That patch might work for me, but I think it papers over the bug where the check
> > > on old->state that you make before sleeping (was COMING || UNFORMED, now !LIVE)
> > > doesn't match the check to wake up in finished_loading().
> > >
> > > The reason the issue might not show up in practice is that your patch basically
> > > polls, so the condition checks in finished_loading() are only a quicker exit.
> > >
> > > If you squash my patch into yours, I think it will cover that case. Though if
> > > polling is the right answer here, it also raises the question of whether or not
> > > we even need finished_loading().
> >
> > The more I look at this I think you're right.  Let me do some additional testing
> > with your patch + my original patch.
>
> I have done testing on arm64, s390x, ppc64le, ppc64, and x86 and have not seen
> any issues.
>
> Jessica, how would you like me to proceed?  Would you like an updated patch with
> Signed-off's from both Barret & myself?

Hi Prarit,

A freshly sent patch with the squashed changes and Signed-off's would
be great. Thank you both for testing and looking into this!

Jessica