Re: [PATCH -next v2 1/6] md: remove useless debug code to print configuration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

在 2023/11/25 0:49, Song Liu 写道:
On Fri, Nov 24, 2023 at 1:12 AM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:

Hi,

在 2023/11/24 16:17, Song Liu 写道:
On Fri, Oct 20, 2023 at 7:25 PM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:

From: Yu Kuai <yukuai3@xxxxxxxxxx>

One the one hand, print_conf() can be called without grabbing
'reconfig_mtuex' and current rcu protection to access rdev through 'conf'
is not safe. Fortunately, there is a separate rcu protection to access
rdev from 'mddev->disks', and rdev is always removed from 'conf' before
'mddev->disks'.

On the other hand, print_conf() is just used for debug,
and user can always grab such information(/proc/mdstat and mdadm).

There is no need to always enable this debug and try to fix misuse rcu
protection for accessing rdev from 'conf', hence remove print_conf().

Signed-off-by: Yu Kuai <yukuai3@xxxxxxxxxx>

I wouldn't call these debug functions useless. There is probably some
users who use them for debugging (or even in some automations).
How hard is it to keep these functions? Can we just add some lockdep
to these functions to make sure they are called from safe places?

Okay, I can keep these debug code, and since these code are
dereferencing rdev from conf, and they need new syncronization:

1) dereference rdev from mddev->disks instead of conf, and use
rdev->raid_disk >= 0 to judge if this rdev is in conf. There might
be a race window that rdev can be removed from conf, however, I think
this dones't matter. Or:

2) grab 'active_io' before print_conf(), to make sure rdev won't be
removed from conf.

I think for most of these cases, we can already do print_conf()
safely (holding mutex/lock), no? We can grab active_io when it is
really necessary.

Ok, I'll look into this, and send a v3 if there are no other corner
cases.

Thanks,
Kuai


Thanks,
Song

.






[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux