lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Tue, 9 Jan 2024 16:28:32 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Song Liu <song@...nel.org>, Yu Kuai <yukuai1@...weicloud.com>
Cc: mariusz.tkaczyk@...ux.intel.com, xni@...hat.com,
 linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org,
 yi.zhang@...wei.com, yangerkun@...wei.com, "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH v3 2/2] md: simplify md_seq_ops

Hi,

在 2024/01/09 16:12, Song Liu 写道:
> On Mon, Jan 8, 2024 at 11:48 PM Yu Kuai <yukuai1@...weicloud.com> wrote:
>>
>> Hi,
>>
>> 在 2024/01/09 9:21, Yu Kuai 写道:
>>> Hi,
>>>
>>> 在 2024/01/09 7:38, Song Liu 写道:
>>>> On Tue, Sep 26, 2023 at 11:19 PM Yu Kuai <yukuai1@...weicloud.com> wrote:
> [...]
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index e351e6c51cc7..289d3d89e73d 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -8135,6 +8135,19 @@ static void status_unused(struct seq_file *seq)
>>           seq_printf(seq, "\n");
>>    }
>>
>> +static void status_personalities(struct seq_file *seq)
>> +{
>> +       struct md_personality *pers;
>> +
>> +       seq_puts(seq, "Personalities : ");
>> +       spin_lock(&pers_lock);
>> +       list_for_each_entry(pers, &pers_list, list)
>> +               seq_printf(seq, "[%s] ", pers->name);
>> +
>> +       spin_unlock(&pers_lock);
>> +       seq_puts(seq, "\n");
>> +}
>> +
>>    static int status_resync(struct seq_file *seq, struct mddev *mddev)
>>    {
>>           sector_t max_sectors, resync, res;
>> @@ -8273,43 +8286,53 @@ static int status_resync(struct seq_file *seq,
>> struct mddev *mddev)
>>           return 1;
>>    }
>>
>> +#define MDDEV_NONE (void *)1
>> +
>>    static void *md_seq_start(struct seq_file *seq, loff_t *pos)
>>           __acquires(&all_mddevs_lock)
>>    {
>> -       struct md_personality *pers;
>> -
>> -       seq_puts(seq, "Personalities : ");
>> -       spin_lock(&pers_lock);
>> -       list_for_each_entry(pers, &pers_list, list)
>> -               seq_printf(seq, "[%s] ", pers->name);
>> -
>> -       spin_unlock(&pers_lock);
>> -       seq_puts(seq, "\n");
>>           seq->poll_event = atomic_read(&md_event_count);
>> -
>>           spin_lock(&all_mddevs_lock);
>>
>> -       return seq_list_start(&all_mddevs, *pos);
>> +       if (!list_empty(&all_mddevs))
>> +               return seq_list_start(&all_mddevs, *pos);
>> +       else if (*pos == 0)
>> +               return MDDEV_NONE;
>> +       else
>> +               return NULL;
>>    }
>>
>>    static void *md_seq_next(struct seq_file *seq, void *v, loff_t *pos)
>>    {
>> +       if (v == MDDEV_NONE) {
>> +               ++*pos;
>> +               return NULL;
>> +       }
>> +
>>           return seq_list_next(v, &all_mddevs, pos);
>>    }
>>
>>    static void md_seq_stop(struct seq_file *seq, void *v)
>>           __releases(&all_mddevs_lock)
>>    {
>> -       status_unused(seq);
>>           spin_unlock(&all_mddevs_lock);
>>    }
>>    static int md_seq_show(struct seq_file *seq, void *v)
>>    {
>> -       struct mddev *mddev = list_entry(v, struct mddev, all_mddevs);
>> +       struct mddev *mddev;
>>           sector_t sectors;
>>           struct md_rdev *rdev;
>>
>> +       if (v == MDDEV_NONE) {
>> +               status_personalities(seq);
>> +               status_unused(seq);
>> +               return 0;
>> +       }
>> +
>> +       mddev = list_entry(v, struct mddev, all_mddevs);
>> +       if (mddev == list_first_entry(&all_mddevs, struct mddev,
>> all_mddevs))
>> +               status_personalities(seq);
>>           if (!mddev_get(mddev))
>>                   return 0;
>>
>> @@ -8385,6 +8408,10 @@ static int md_seq_show(struct seq_file *seq, void *v)
>>           }
>>           spin_unlock(&mddev->lock);
>>           spin_lock(&all_mddevs_lock);
>> +
>> +       if (mddev == list_last_entry(&all_mddevs, struct mddev, all_mddevs))
>> +               status_unused(seq);
>> +
>>           if (atomic_dec_and_test(&mddev->active))
>>                   __mddev_put(mddev);
>>
> 
> I think something like the following is the right way to do this.
> 
> Thanks,
> Song
> 
> diff --git i/drivers/md/md.c w/drivers/md/md.c
> index 38a6767c65b1..14044febe009 100644
> --- i/drivers/md/md.c
> +++ w/drivers/md/md.c
> @@ -8215,20 +8215,8 @@ static int status_resync(struct seq_file *seq,
> struct mddev *mddev)
>   static void *md_seq_start(struct seq_file *seq, loff_t *pos)
>          __acquires(&all_mddevs_lock)
>   {
> -       struct md_personality *pers;
> -
> -       seq_puts(seq, "Personalities : ");
> -       spin_lock(&pers_lock);
> -       list_for_each_entry(pers, &pers_list, list)
> -               seq_printf(seq, "[%s] ", pers->name);
> -
> -       spin_unlock(&pers_lock);
> -       seq_puts(seq, "\n");
> -       seq->poll_event = atomic_read(&md_event_count);
> -
>          spin_lock(&all_mddevs_lock);
> -
> -       return seq_list_start(&all_mddevs, *pos);
> +       return seq_list_start_head(&all_mddevs, *pos);

Yes, this is good. I didn't notice the api seq_list_start_head().
>   }
> 
>   static void *md_seq_next(struct seq_file *seq, void *v, loff_t *pos)
> @@ -8243,12 +8231,31 @@ static void md_seq_stop(struct seq_file *seq, void *v)
>          spin_unlock(&all_mddevs_lock);
>   }
> 
> +static void md_seq_print_header(struct seq_file *seq)
> +{
> +       struct md_personality *pers;
> +
> +       seq_puts(seq, "Personalities : ");
> +       spin_lock(&pers_lock);
> +       list_for_each_entry(pers, &pers_list, list)
> +               seq_printf(seq, "[%s] ", pers->name);
> +
> +       spin_unlock(&pers_lock);
> +       seq_puts(seq, "\n");
> +       seq->poll_event = atomic_read(&md_event_count);
> +}
> +
>   static int md_seq_show(struct seq_file *seq, void *v)
>   {
>          struct mddev *mddev = list_entry(v, struct mddev, all_mddevs);
>          sector_t sectors;
>          struct md_rdev *rdev;
> 
> +       if (v == &all_mddevs) {
> +               md_seq_print_header(seq);

And I will still move status_unused() to md_seq_show(), because 
seq_read_iter() only handle the case that seq_printf() overflowed from
md_seq_show(), not md_seq_start/stop().

Thanks,
Kuai

> +               return 0;
> +       }
> +
>          if (!mddev_get(mddev))
>                  return 0;
> 
> .
> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ