Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1371216 > unrolled thread

Re: [PATCH v2] parport: register driver later

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2] parport: register driver later Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-04-05 07:30 +0200
    Re: [PATCH v2] parport: register driver later Greg KH <gregkh@linuxfoundation.org> - 2016-04-05 15:00 +0200
      Re: [PATCH v2] parport: register driver later Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-04-05 15:20 +0200
        Re: [PATCH v2] parport: register driver later Greg KH <gregkh@linuxfoundation.org> - 2016-04-05 15:50 +0200
      Re: [PATCH v2] parport: register driver later Greg KH <gregkh@linuxfoundation.org> - 2016-04-05 15:50 +0200
        Re: [PATCH v2] parport: register driver later Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-05 16:10 +0200
      Re: [PATCH v2] parport: register driver later Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-05 15:50 +0200

#1371216 — Re: [PATCH v2] parport: register driver later

On Mon, Mar 07, 2016 at 10:32:55AM -0700, Ross Zwisler wrote:
> On Sun, Mar 06, 2016 at 08:40:10PM +0530, Sudip Mukherjee wrote:
> > If the parport bus is not yet registered and any device using parallel
> > port tries to register with the bus we get a stackdump with a message
> > of Kernel bug.
> > 
> > Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> > Cc: <stable@vger.kernel.org> # 4.2+
> > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> > ---
> > 
> > Hi Ross,
> > Can you please test this patch in your setup. This is a respin of the
> > previous patch in another way.
> 
> Yep, this also solves the issue for me.
> 
> Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>

Hi Greg,
If this patch is ok, can we please have it in v4.6 .
Anyway, the problem patch which this patch tried to fix has already
been reverted by Linus - 
1701f680407c ("Revert "ppdev: use new parport device model"") but we still
can have problem with the other devices that use parport.

BTW, I know you are busy, but in these situations where I need to have
the fix urgently in the tree, is there any other way to solve the purpose?
I feel it was incompetency on my part where Linus had to interfere and
revert a patch even though the fix was already posted.

regards
sudip

[toc] | [next] | [standalone]

#1371542

On Tue, Apr 05, 2016 at 06:26:08AM +0100, Sudip Mukherjee wrote:
> On Mon, Mar 07, 2016 at 10:32:55AM -0700, Ross Zwisler wrote:
> > On Sun, Mar 06, 2016 at 08:40:10PM +0530, Sudip Mukherjee wrote:
> > > If the parport bus is not yet registered and any device using parallel
> > > port tries to register with the bus we get a stackdump with a message
> > > of Kernel bug.
> > > 
> > > Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> > > Cc: <stable@vger.kernel.org> # 4.2+
> > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> > > ---
> > > 
> > > Hi Ross,
> > > Can you please test this patch in your setup. This is a respin of the
> > > previous patch in another way.
> > 
> > Yep, this also solves the issue for me.
> > 
> > Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> 
> Hi Greg,
> If this patch is ok, can we please have it in v4.6 .
> Anyway, the problem patch which this patch tried to fix has already
> been reverted by Linus - 
> 1701f680407c ("Revert "ppdev: use new parport device model"") but we still
> can have problem with the other devices that use parport.
> 
> BTW, I know you are busy, but in these situations where I need to have
> the fix urgently in the tree, is there any other way to solve the purpose?
> I feel it was incompetency on my part where Linus had to interfere and
> revert a patch even though the fix was already posted.

A bit better commit message here would have caused me to notice it.
Something like "Revert a broken patch because it crashes all of our
machines without it!!!" would be a hint it needed to go in :)

I think the lack of parport hardware around seems to have caused a total
lack of testing this code path while it was in linux-next and in my
local testing, sorry about that, it should have been caught a lot
earlier.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1371557

On Tuesday 05 April 2016 06:28 PM, Greg KH wrote:
> On Tue, Apr 05, 2016 at 06:26:08AM +0100, Sudip Mukherjee wrote:
>> On Mon, Mar 07, 2016 at 10:32:55AM -0700, Ross Zwisler wrote:
>>> On Sun, Mar 06, 2016 at 08:40:10PM +0530, Sudip Mukherjee wrote:
>>>> If the parport bus is not yet registered and any device using parallel
>>>> port tries to register with the bus we get a stackdump with a message
>>>> of Kernel bug.
>>>>
>>>> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
>>>> Cc: <stable@vger.kernel.org> # 4.2+
>>>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
>>>> ---
>>>>
>>>> Hi Ross,
>>>> Can you please test this patch in your setup. This is a respin of the
>>>> previous patch in another way.
>>>
>>> Yep, this also solves the issue for me.
>>>
>>> Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>>
>> Hi Greg,
>> If this patch is ok, can we please have it in v4.6 .
>> Anyway, the problem patch which this patch tried to fix has already
>> been reverted by Linus -
>> 1701f680407c ("Revert "ppdev: use new parport device model"") but we still
>> can have problem with the other devices that use parport.
>>
>> BTW, I know you are busy, but in these situations where I need to have
>> the fix urgently in the tree, is there any other way to solve the purpose?
>> I feel it was incompetency on my part where Linus had to interfere and
>> revert a patch even though the fix was already posted.
>
> A bit better commit message here would have caused me to notice it.
> Something like "Revert a broken patch because it crashes all of our
> machines without it!!!" would be a hint it needed to go in :)

Well. its actually my fault. Previously I used to ping and remind you if 
there is something urgent which needed to go in before your tree closes. 
But this time I got busy with the job change and traveling and 
everything was a mess on my side.
So, now that the ppdev patch has been reverted by Linus, what do you 
suggest that we do? I will say, to have this patch as a temporary fix 
(for other devices) while I work on the deferred probe for parport which 
will solve the problem.

regards
sudip

[toc] | [prev] | [next] | [standalone]

#1371609

On Tue, Apr 05, 2016 at 06:47:36PM +0530, Sudip Mukherjee wrote:
> On Tuesday 05 April 2016 06:28 PM, Greg KH wrote:
> > On Tue, Apr 05, 2016 at 06:26:08AM +0100, Sudip Mukherjee wrote:
> > > On Mon, Mar 07, 2016 at 10:32:55AM -0700, Ross Zwisler wrote:
> > > > On Sun, Mar 06, 2016 at 08:40:10PM +0530, Sudip Mukherjee wrote:
> > > > > If the parport bus is not yet registered and any device using parallel
> > > > > port tries to register with the bus we get a stackdump with a message
> > > > > of Kernel bug.
> > > > > 
> > > > > Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> > > > > Cc: <stable@vger.kernel.org> # 4.2+
> > > > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> > > > > ---
> > > > > 
> > > > > Hi Ross,
> > > > > Can you please test this patch in your setup. This is a respin of the
> > > > > previous patch in another way.
> > > > 
> > > > Yep, this also solves the issue for me.
> > > > 
> > > > Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > 
> > > Hi Greg,
> > > If this patch is ok, can we please have it in v4.6 .
> > > Anyway, the problem patch which this patch tried to fix has already
> > > been reverted by Linus -
> > > 1701f680407c ("Revert "ppdev: use new parport device model"") but we still
> > > can have problem with the other devices that use parport.
> > > 
> > > BTW, I know you are busy, but in these situations where I need to have
> > > the fix urgently in the tree, is there any other way to solve the purpose?
> > > I feel it was incompetency on my part where Linus had to interfere and
> > > revert a patch even though the fix was already posted.
> > 
> > A bit better commit message here would have caused me to notice it.
> > Something like "Revert a broken patch because it crashes all of our
> > machines without it!!!" would be a hint it needed to go in :)
> 
> Well. its actually my fault. Previously I used to ping and remind you if
> there is something urgent which needed to go in before your tree closes. But
> this time I got busy with the job change and traveling and everything was a
> mess on my side.
> So, now that the ppdev patch has been reverted by Linus, what do you suggest
> that we do? I will say, to have this patch as a temporary fix (for other
> devices) while I work on the deferred probe for parport which will solve the
> problem.

Work on deferred probe, you should have time before the 4.7-rc1 merge
window happens.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1371608

On Tue, Apr 05, 2016 at 06:44:12AM -0700, Linus Torvalds wrote:
> On Tue, Apr 5, 2016 at 5:58 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > A bit better commit message here would have caused me to notice it.
> > Something like "Revert a broken patch because it crashes all of our
> > machines without it!!!" would be a hint it needed to go in :)
> 
> Didn't my revert of e7223f1860 fix this?

Yes.

> > I think the lack of parport hardware around seems to have caused a total
> > lack of testing this code path while it was in linux-next and in my
> > local testing, sorry about that, it should have been caught a lot
> > earlier.
> 
> Well, the original 0day kernel test robot report was from the
> linux-next days, back in February. I reverted the commit that the
> kernel test robot indicated was the point where the actual trouble
> started two weeks ago (commit 1701f680407c).
> 
> I was hoping that would be it. Have there been reports since that I
> haven't seen?

Not that I have seen.  But I missed the 0day report so I might not be
the best judge here...

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1371637

On Tue, Apr 5, 2016 at 6:47 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>>
>> I was hoping that would be it. Have there been reports since that I
>> haven't seen?
>
> Not that I have seen.

Ok. So as far as I can see we can ignore this for now (at at least be
no worse off than we used to be).

I didn't check *why* ppdev_init happens before the parport driver has
been initialized,

I get the feeling that the trivial fix would be to just make a new
"parport_init/exit()" pair that just does the parport_bus_init(). And
mark that as "subsys_initcall()" so that it gets done before the
individual drivers. No need to even be fancy about deferred probing
etc, just a simple "ppdev depends on parport" chain.

But I didn't look into the actual sequence of events, so it's entirely
possible I'm missing something.

                 Linus

[toc] | [prev] | [next] | [standalone]

#1371610

On Tue, Apr 5, 2016 at 5:58 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> A bit better commit message here would have caused me to notice it.
> Something like "Revert a broken patch because it crashes all of our
> machines without it!!!" would be a hint it needed to go in :)

Didn't my revert of e7223f1860 fix this?

> I think the lack of parport hardware around seems to have caused a total
> lack of testing this code path while it was in linux-next and in my
> local testing, sorry about that, it should have been caught a lot
> earlier.

Well, the original 0day kernel test robot report was from the
linux-next days, back in February. I reverted the commit that the
kernel test robot indicated was the point where the actual trouble
started two weeks ago (commit 1701f680407c).

I was hoping that would be it. Have there been reports since that I
haven't seen?

                Linus

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web