Re: [PATCH v2 01/46] mtdpart: Propagate _get/put_device()

Re: [PATCH v2 01/46] mtdpart: Propagate _get/put_device()

[Karl Beldan]

On Wed, Sep 28, 2016 at 8:16 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Wed, Sep 21, 2016 at 12:15:31PM +0200, Boris Brezillon wrote:
>> On Wed, 21 Sep 2016 11:43:56 +0200
>> Daniel Walter <dwalter@sigma-star.at> wrote:
>>
>> > From: Richard Weinberger <richard@nod.at>
>> >
>> > If the master device has callbacks for _get/put_device()
>> > and this MTD has slaves a get_mtd_device() call on paritions
>> > will never issue the registered callbacks.
>> > Fix this by propagating _get/put_device() down.
>>
>> Brian, can we have this one queued for 4.9? I can't take it in my tree
>> if you want, but it's probably better if it's in the mtd tree.
>
> Applied this patch to l2-mtd.git
>

I think this should also go into -stable.

Re: [PATCH v2 01/46] mtdpart: Propagate _get/put_device()

[Brian Norris]

On Wed, Dec 14, 2016 at 07:24:46PM +0000, Karl Beldan wrote:
> On Wed, Sep 28, 2016 at 8:16 PM, Brian Norris
> <computersforpeace@gmail.com> wrote:
> > On Wed, Sep 21, 2016 at 12:15:31PM +0200, Boris Brezillon wrote:
> >> On Wed, 21 Sep 2016 11:43:56 +0200
> >> Daniel Walter <dwalter@sigma-star.at> wrote:
> >>
> >> > From: Richard Weinberger <richard@nod.at>
> >> >
> >> > If the master device has callbacks for _get/put_device()
> >> > and this MTD has slaves a get_mtd_device() call on paritions
> >> > will never issue the registered callbacks.
> >> > Fix this by propagating _get/put_device() down.
> >>
> >> Brian, can we have this one queued for 4.9? I can't take it in my tree
> >> if you want, but it's probably better if it's in the mtd tree.
> >
> > Applied this patch to l2-mtd.git
> >
> 
> I think this should also go into -stable.

Why? Do you have real use cases that are broken by this? I understand
this is a problem, but I'm curious on how this satisfies the stable
rules.

Also, note that this isn't a regression; it's been broken forever and
apparently no one noticed. IMO that raises the bar a bit (but not
impossibly so) for -stable.

Anyway, if we decide to do this, you'll also want to include the git
hash and applicable kernel versions, per Option 2 [1].

Brian

[1] Documentation/stable_kernel_rules.txt.

Re: [PATCH v2 01/46] mtdpart: Propagate _get/put_device()

[Richard Weinberger]

Hi!

On 14.12.2016 22:09, Brian Norris wrote:
> On Wed, Dec 14, 2016 at 07:24:46PM +0000, Karl Beldan wrote:
>> On Wed, Sep 28, 2016 at 8:16 PM, Brian Norris
>> <computersforpeace@gmail.com> wrote:
>>> On Wed, Sep 21, 2016 at 12:15:31PM +0200, Boris Brezillon wrote:
>>>> On Wed, 21 Sep 2016 11:43:56 +0200
>>>> Daniel Walter <dwalter@sigma-star.at> wrote:
>>>>
>>>>> From: Richard Weinberger <richard@nod.at>
>>>>>
>>>>> If the master device has callbacks for _get/put_device()
>>>>> and this MTD has slaves a get_mtd_device() call on paritions
>>>>> will never issue the registered callbacks.
>>>>> Fix this by propagating _get/put_device() down.
>>>>
>>>> Brian, can we have this one queued for 4.9? I can't take it in my tree
>>>> if you want, but it's probably better if it's in the mtd tree.
>>>
>>> Applied this patch to l2-mtd.git
>>>
>>
>> I think this should also go into -stable.
> 
> Why? Do you have real use cases that are broken by this? I understand
> this is a problem, but I'm curious on how this satisfies the stable
> rules.
> 
> Also, note that this isn't a regression; it's been broken forever and
> apparently no one noticed. IMO that raises the bar a bit (but not
> impossibly so) for -stable.

Yes. AFAICT you can only trigger it using my "new" nandsim
which is not mainline so far.

Thanks,
//richard

Re: [PATCH v2 01/46] mtdpart: Propagate _get/put_device()

[Brian Norris]

On Wed, Dec 14, 2016 at 10:12:42PM +0100, Richard Weinberger wrote:
> On 14.12.2016 22:09, Brian Norris wrote:
> > Also, note that this isn't a regression; it's been broken forever and
> > apparently no one noticed. IMO that raises the bar a bit (but not
> > impossibly so) for -stable.
> 
> Yes. AFAICT you can only trigger it using my "new" nandsim
> which is not mainline so far.

Ah, OK. So the only current _{get,put}_device() implementor in mainline
is gluebi, so far. And it's OK for now to just have the master device be
refcounted, and just rely on the partitions being removed before the
master, right?

In that case, no, this shouldn't go to -stable.

Brian

Re: [PATCH v2 01/46] mtdpart: Propagate _get/put_device()

[Karl Beldan]

On Wed, Dec 14, 2016 at 9:09 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Wed, Dec 14, 2016 at 07:24:46PM +0000, Karl Beldan wrote:
>> On Wed, Sep 28, 2016 at 8:16 PM, Brian Norris
>> <computersforpeace@gmail.com> wrote:
>> > On Wed, Sep 21, 2016 at 12:15:31PM +0200, Boris Brezillon wrote:
>> >> On Wed, 21 Sep 2016 11:43:56 +0200
>> >> Daniel Walter <dwalter@sigma-star.at> wrote:
>> >>
>> >> > From: Richard Weinberger <richard@nod.at>
>> >> >
>> >> > If the master device has callbacks for _get/put_device()
>> >> > and this MTD has slaves a get_mtd_device() call on paritions
>> >> > will never issue the registered callbacks.
>> >> > Fix this by propagating _get/put_device() down.
>> >>
>> >> Brian, can we have this one queued for 4.9? I can't take it in my tree
>> >> if you want, but it's probably better if it's in the mtd tree.
>> >
>> > Applied this patch to l2-mtd.git
>> >
>>
>> I think this should also go into -stable.
>
> Why? Do you have real use cases that are broken by this? I understand

I do, some code adding partitions on a gluebi master.

> this is a problem, but I'm curious on how this satisfies the stable
> rules.
>
> Also, note that this isn't a regression; it's been broken forever and
> apparently no one noticed. IMO that raises the bar a bit (but not
> impossibly so) for -stable.
>

I just encountered the bug yesterday and yes it is obvious it has been
broken forever.
I don't have strong opinion about these things so no worries.

Karl

> Anyway, if we decide to do this, you'll also want to include the git
> hash and applicable kernel versions, per Option 2 [1].
>
> Brian
>
> [1] Documentation/stable_kernel_rules.txt.

Re: [PATCH v2 01/46] mtdpart: Propagate _get/put_device()

[Richard Weinberger]

On 15.12.2016 08:09, Karl Beldan wrote:
>>> I think this should also go into -stable.
>>
>> Why? Do you have real use cases that are broken by this? I understand
> 
> I do, some code adding partitions on a gluebi master.

What exactly are you doing?

>> this is a problem, but I'm curious on how this satisfies the stable
>> rules.
>>
>> Also, note that this isn't a regression; it's been broken forever and
>> apparently no one noticed. IMO that raises the bar a bit (but not
>> impossibly so) for -stable.
>>
> 
> I just encountered the bug yesterday and yes it is obvious it has been
> broken forever.
> I don't have strong opinion about these things so no worries.

If existing stuff is broken, and you can trigger it. Please let us
know. Then it should go into -stable.

Thanks,
//richard

Re: [PATCH v2 01/46] mtdpart: Propagate _get/put_device()

[Karl Beldan]

On Thu, Dec 15, 2016 at 08:51:06AM +0100, Richard Weinberger wrote:
> On 15.12.2016 08:09, Karl Beldan wrote:
> >>> I think this should also go into -stable.
> >>
> >> Why? Do you have real use cases that are broken by this? I understand
> > 
> > I do, some code adding partitions on a gluebi master.
> 
> What exactly are you doing?
> 
> >> this is a problem, but I'm curious on how this satisfies the stable
> >> rules.
> >>
> >> Also, note that this isn't a regression; it's been broken forever and
> >> apparently no one noticed. IMO that raises the bar a bit (but not
> >> impossibly so) for -stable.
> >>
> > 
> > I just encountered the bug yesterday and yes it is obvious it has been
> > broken forever.
> > I don't have strong opinion about these things so no worries.
> 
> If existing stuff is broken, and you can trigger it. Please let us
> know. Then it should go into -stable.
> 

I thought that's what I already did.

Anyways, it didn't require much imagination to come up with a script
triggering the issue so here you are:

#{{

modprobe nandsim
modprobe gluebi
ubiattach -p /dev/mtd1 -b 1
ubimkvol /dev/ubi0 -S 4 -N vol_0
mtdpart add /dev/mtd2 vol_0_0 0 0x1000
tail -F /dev/mtd2 &
while :; do dd bs=1 count=1 if=/dev/mtd3 >/dev/null 2>&1 || break; done
kill -9 %1 # Oops

#}}

 
Karl