mbox series

[v3,0/4] btrf_show_devname related fixes

Message ID cover.1629458519.git.anand.jain@oracle.com (mailing list archive)
Headers show
Series btrf_show_devname related fixes | expand

Message

Anand Jain Aug. 20, 2021, 11:28 a.m. UTC
These fixes are inspired by the bug report and its discussions in the
mailing list subject
 btrfs: traverse seed devices if fs_devices::devices is empty in show_devname

v3:
 Fix rcu_lock in the patch 3/4

Anand Jain (4):
  btrfs: consolidate device_list_mutex in prepare_sprout to its parent
  btrfs: save latest btrfs_device instead of its block_device in
    fs_devices
  btrfs: use latest_dev in btrfs_show_devname
  btrfs: update latest_dev when we sprout

 fs/btrfs/disk-io.c   |  6 +++---
 fs/btrfs/extent_io.c |  2 +-
 fs/btrfs/inode.c     |  2 +-
 fs/btrfs/procfs.c    |  6 +++---
 fs/btrfs/super.c     | 26 ++++----------------------
 fs/btrfs/volumes.c   | 19 +++++++++++--------
 fs/btrfs/volumes.h   |  2 +-
 7 files changed, 24 insertions(+), 39 deletions(-)

Comments

Anand Jain Aug. 20, 2021, 11:32 a.m. UTC | #1
Please consider patches 1/4 and 3/4 as RFC.
The reason for the RFC is in the individual patches.

Thanks.

On 20/08/2021 19:28, Anand Jain wrote:
> These fixes are inspired by the bug report and its discussions in the
> mailing list subject
>   btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
> 
> v3:
>   Fix rcu_lock in the patch 3/4
> 
> Anand Jain (4):
>    btrfs: consolidate device_list_mutex in prepare_sprout to its parent
>    btrfs: save latest btrfs_device instead of its block_device in
>      fs_devices
>    btrfs: use latest_dev in btrfs_show_devname
>    btrfs: update latest_dev when we sprout
> 
>   fs/btrfs/disk-io.c   |  6 +++---
>   fs/btrfs/extent_io.c |  2 +-
>   fs/btrfs/inode.c     |  2 +-
>   fs/btrfs/procfs.c    |  6 +++---
>   fs/btrfs/super.c     | 26 ++++----------------------
>   fs/btrfs/volumes.c   | 19 +++++++++++--------
>   fs/btrfs/volumes.h   |  2 +-
>   7 files changed, 24 insertions(+), 39 deletions(-)
>
David Sterba Aug. 23, 2021, 11:45 a.m. UTC | #2
On Fri, Aug 20, 2021 at 07:28:38PM +0800, Anand Jain wrote:
> These fixes are inspired by the bug report and its discussions in the
> mailing list subject
>  btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
> 
> v3:
>  Fix rcu_lock in the patch 3/4

Please next time send a separate patch with another revision, the
threading is a total mess, look

https://lore.kernel.org/linux-btrfs/7171ca39-8f57-4646-face-70d3d23fbf02@oracle.com/T/#t
David Sterba Aug. 23, 2021, 7:46 p.m. UTC | #3
On Mon, Aug 23, 2021 at 07:31:38PM +0800, Anand Jain wrote:
> These fixes are inspired by the bug report and its discussions in the
> mailing list subject
>  btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
> 
> And depends on the patch
>  [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs
> in the ML
> 
> v4:
>  Fix unrelated changes in 2/4
> v3:
>  Fix rcu_lock in the patch 3/4
> 
> Anand Jain (4):
>   btrfs: consolidate device_list_mutex in prepare_sprout to its parent
>   btrfs: save latest btrfs_device instead of its block_device in
>     fs_devices
>   btrfs: use latest_dev in btrfs_show_devname
>   btrfs: update latest_dev when we sprout

Patchset survived one round of fstests and I haven't seen the lockdep
warning in btrfs/020 that's caused by some unrelated work in loop device
driver. There's a series from Josef to fix it by shuffling locking, so
it would be interesting to see where's the difference.
Anand Jain Aug. 24, 2021, 12:28 a.m. UTC | #4
On 24/08/2021 03:46, David Sterba wrote:
> On Mon, Aug 23, 2021 at 07:31:38PM +0800, Anand Jain wrote:
>> These fixes are inspired by the bug report and its discussions in the
>> mailing list subject
>>   btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
>>
>> And depends on the patch
>>   [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs
>> in the ML
>>
>> v4:
>>   Fix unrelated changes in 2/4
>> v3:
>>   Fix rcu_lock in the patch 3/4
>>
>> Anand Jain (4):
>>    btrfs: consolidate device_list_mutex in prepare_sprout to its parent
>>    btrfs: save latest btrfs_device instead of its block_device in
>>      fs_devices
>>    btrfs: use latest_dev in btrfs_show_devname
>>    btrfs: update latest_dev when we sprout
> 
> Patchset survived one round of fstests and I haven't seen the lockdep
> warning in btrfs/020 that's caused by some unrelated work in loop device
> driver.


> There's a series from Josef to fix it by shuffling locking,

  Hm. Is it a recent patch? I can't find.

  Is it about device_list_mutex (as in cleanup patch 1 above) or 
btrfs_show_devname() (which patches 2-4 fixes)?
  Yeah, patch 1 is unrelated to patches 2-4 will separate them send v5.



> so
> it would be interesting to see where's the difference.
David Sterba Aug. 24, 2021, 4:11 p.m. UTC | #5
On Tue, Aug 24, 2021 at 08:28:09AM +0800, Anand Jain wrote:
> 
> 
> On 24/08/2021 03:46, David Sterba wrote:
> > On Mon, Aug 23, 2021 at 07:31:38PM +0800, Anand Jain wrote:
> >> These fixes are inspired by the bug report and its discussions in the
> >> mailing list subject
> >>   btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
> >>
> >> And depends on the patch
> >>   [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs
> >> in the ML
> >>
> >> v4:
> >>   Fix unrelated changes in 2/4
> >> v3:
> >>   Fix rcu_lock in the patch 3/4
> >>
> >> Anand Jain (4):
> >>    btrfs: consolidate device_list_mutex in prepare_sprout to its parent
> >>    btrfs: save latest btrfs_device instead of its block_device in
> >>      fs_devices
> >>    btrfs: use latest_dev in btrfs_show_devname
> >>    btrfs: update latest_dev when we sprout
> > 
> > Patchset survived one round of fstests and I haven't seen the lockdep
> > warning in btrfs/020 that's caused by some unrelated work in loop device
> > driver.
> 
> 
> > There's a series from Josef to fix it by shuffling locking,
> 
>   Hm. Is it a recent patch? I can't find.

https://lore.kernel.org/linux-btrfs/cover.1627419595.git.josef@toxicpanda.com/

>   Is it about device_list_mutex (as in cleanup patch 1 above) or 
> btrfs_show_devname() (which patches 2-4 fixes)?

Relatd to device locking, so device list mutex and also uuid mutex IIRC.
Anand Jain Aug. 25, 2021, 2:13 a.m. UTC | #6
On 25/08/2021 00:11, David Sterba wrote:
> On Tue, Aug 24, 2021 at 08:28:09AM +0800, Anand Jain wrote:
>>
>>
>> On 24/08/2021 03:46, David Sterba wrote:
>>> On Mon, Aug 23, 2021 at 07:31:38PM +0800, Anand Jain wrote:
>>>> These fixes are inspired by the bug report and its discussions in the
>>>> mailing list subject
>>>>    btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
>>>>
>>>> And depends on the patch
>>>>    [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs
>>>> in the ML
>>>>
>>>> v4:
>>>>    Fix unrelated changes in 2/4
>>>> v3:
>>>>    Fix rcu_lock in the patch 3/4
>>>>
>>>> Anand Jain (4):
>>>>     btrfs: consolidate device_list_mutex in prepare_sprout to its parent
>>>>     btrfs: save latest btrfs_device instead of its block_device in
>>>>       fs_devices
>>>>     btrfs: use latest_dev in btrfs_show_devname
>>>>     btrfs: update latest_dev when we sprout
>>>
>>> Patchset survived one round of fstests and I haven't seen the lockdep
>>> warning in btrfs/020 that's caused by some unrelated work in loop device
>>> driver.
>>
>>
>>> There's a series from Josef to fix it by shuffling locking,
>>
>>    Hm. Is it a recent patch? I can't find.
> 
> https://lore.kernel.org/linux-btrfs/cover.1627419595.git.josef@toxicpanda.com/

Thx.
It is the same fix I sent ~3months before in the ML[1].
If you want, I can refresh [1] and include 1/4 (in this patchset)
into a separate patchset.

[1]
https://patchwork.kernel.org/project/linux-btrfs/patch/23a8830f3be500995e74b45f18862e67c0634c3d.1614793362.git.anand.jain@oracle.com/

> 
>>    Is it about device_list_mutex (as in cleanup patch 1 above) or
>> btrfs_show_devname() (which patches 2-4 fixes)?
> 
> Relatd to device locking, so device list mutex and also uuid mutex IIRC.
>
David Sterba Aug. 31, 2021, 3:40 p.m. UTC | #7
On Wed, Aug 25, 2021 at 10:13:52AM +0800, Anand Jain wrote:
> On 25/08/2021 00:11, David Sterba wrote:
> > On Tue, Aug 24, 2021 at 08:28:09AM +0800, Anand Jain wrote:
> >>
> >>
> >> On 24/08/2021 03:46, David Sterba wrote:
> >>> On Mon, Aug 23, 2021 at 07:31:38PM +0800, Anand Jain wrote:
> >>>> These fixes are inspired by the bug report and its discussions in the
> >>>> mailing list subject
> >>>>    btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
> >>>>
> >>>> And depends on the patch
> >>>>    [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs
> >>>> in the ML
> >>>>
> >>>> v4:
> >>>>    Fix unrelated changes in 2/4
> >>>> v3:
> >>>>    Fix rcu_lock in the patch 3/4
> >>>>
> >>>> Anand Jain (4):
> >>>>     btrfs: consolidate device_list_mutex in prepare_sprout to its parent
> >>>>     btrfs: save latest btrfs_device instead of its block_device in
> >>>>       fs_devices
> >>>>     btrfs: use latest_dev in btrfs_show_devname
> >>>>     btrfs: update latest_dev when we sprout
> >>>
> >>> Patchset survived one round of fstests and I haven't seen the lockdep
> >>> warning in btrfs/020 that's caused by some unrelated work in loop device
> >>> driver.
> >>
> >>
> >>> There's a series from Josef to fix it by shuffling locking,
> >>
> >>    Hm. Is it a recent patch? I can't find.
> > 
> > https://lore.kernel.org/linux-btrfs/cover.1627419595.git.josef@toxicpanda.com/
> 
> Thx.
> It is the same fix I sent ~3months before in the ML[1].

Well, I've seen the patch and pings but sometimes the timing is not good
and there's something else I have to finish before looking into new
area. And device locking is something that needs full attention. The
bugs/warnings in question weren't that important.

But anyway now the 5.15 branch is out and there are several patches
regarding the devices so I'm going to spend time on that.  Also because
the recent updates to loop devices started to trigger lockdep warnings
that make testing harder and we're missing lockdep in many tests.
Anand Jain Sept. 1, 2021, 8:18 a.m. UTC | #8
On 31/08/2021 23:40, David Sterba wrote:
> On Wed, Aug 25, 2021 at 10:13:52AM +0800, Anand Jain wrote:
>> On 25/08/2021 00:11, David Sterba wrote:
>>> On Tue, Aug 24, 2021 at 08:28:09AM +0800, Anand Jain wrote:
>>>>
>>>>
>>>> On 24/08/2021 03:46, David Sterba wrote:
>>>>> On Mon, Aug 23, 2021 at 07:31:38PM +0800, Anand Jain wrote:
>>>>>> These fixes are inspired by the bug report and its discussions in the
>>>>>> mailing list subject
>>>>>>     btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
>>>>>>
>>>>>> And depends on the patch
>>>>>>     [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs
>>>>>> in the ML
>>>>>>
>>>>>> v4:
>>>>>>     Fix unrelated changes in 2/4
>>>>>> v3:
>>>>>>     Fix rcu_lock in the patch 3/4
>>>>>>
>>>>>> Anand Jain (4):
>>>>>>      btrfs: consolidate device_list_mutex in prepare_sprout to its parent
>>>>>>      btrfs: save latest btrfs_device instead of its block_device in
>>>>>>        fs_devices
>>>>>>      btrfs: use latest_dev in btrfs_show_devname
>>>>>>      btrfs: update latest_dev when we sprout
>>>>>
>>>>> Patchset survived one round of fstests and I haven't seen the lockdep
>>>>> warning in btrfs/020 that's caused by some unrelated work in loop device
>>>>> driver.
>>>>
>>>>
>>>>> There's a series from Josef to fix it by shuffling locking,
>>>>
>>>>     Hm. Is it a recent patch? I can't find.
>>>
>>> https://lore.kernel.org/linux-btrfs/cover.1627419595.git.josef@toxicpanda.com/
>>
>> Thx.
>> It is the same fix I sent ~3months before in the ML[1].
> 
> Well, I've seen the patch and pings but sometimes the timing is not good
> and there's something else I have to finish before looking into new
> area. And device locking is something that needs full attention. The
> bugs/warnings in question weren't that important.

  Ah. Thanks for taking the time to explain.

> But anyway now the 5.15 branch is out and there are several patches
> regarding the devices so I'm going to spend time on that.

  Thx. That includes read_policy patchset as well? Let me know so that I
  can refresh/resend.

>  Also because
> the recent updates to loop devices started to trigger lockdep warnings
> that make testing harder and we're missing lockdep in many tests.

  I am reviewing the patch 1 and 2 in Josef list.
David Sterba Sept. 1, 2021, 4:16 p.m. UTC | #9
On Wed, Sep 01, 2021 at 04:18:41PM +0800, Anand Jain wrote:
> > But anyway now the 5.15 branch is out and there are several patches
> > regarding the devices so I'm going to spend time on that.
> 
>   Thx. That includes read_policy patchset as well? Let me know so that I
>   can refresh/resend.

Yeah we should finalize that, we have all the building blocks ready.
What's remaining is proper benchmarking, IIRC we haven't found a good
default policy yet.

> 
> >  Also because
> > the recent updates to loop devices started to trigger lockdep warnings
> > that make testing harder and we're missing lockdep in many tests.
> 
>   I am reviewing the patch 1 and 2 in Josef list.

Thanks, reviewing the whole batch should be easier now.