From: Nikolay Borisov <nborisov@suse.com>
To: Anand Jain <anand.jain@oracle.com>, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 4/6] btrfs: make btrfs_free_stale_device() argument optional
Date: Sat, 16 Dec 2017 08:55:53 +0200 [thread overview]
Message-ID: <d6ac16e5-2f11-ae8f-b5cd-97ef71e94b2d@suse.com> (raw)
In-Reply-To: <ad4808f1-1ab4-df47-a45a-f3bf6837b813@oracle.com>
On 16.12.2017 04:14, Anand Jain wrote:
> /*
>>> @@ -637,8 +632,11 @@ static void btrfs_free_stale_device(struct
>>> btrfs_device *cur_dev)
>>> * either use mapper or non mapper path throughout.
>>> */
>>> rcu_read_lock();
>>> - not_found = strcmp(rcu_str_deref(dev->name),
>>> - rcu_str_deref(cur_dev->name));
>>> + if (cur_dev)
>>> + not_found = strcmp(rcu_str_deref(dev->name),
>>> + rcu_str_deref(cur_dev->name));
>>> + else
>>> + not_found = 0;
>>
>> nit: Perhaps put a proper documentation header at the beginning of the
>> function detailing this behavior. I.e.
>>
>> /*
>> * btrfs_free_stale_device
>> * ....
>>
>
> Right. Will add.
I posted this comment before reading your other patches, having done
that I'd say it's best to add the documentation once all the cleanups
have been done, since you also add another semantics with the "path"
argument.
>
> Thanks, Anand
>
>
next prev parent reply other threads:[~2017-12-16 6:55 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-15 3:47 [PATCH 0/6] preparatory work to add device forget Anand Jain
2017-12-15 3:47 ` [PATCH 1/6] btrfs: cleanup btrfs_free_stale_device() usage Anand Jain
2017-12-15 15:33 ` Nikolay Borisov
2017-12-16 2:15 ` Anand Jain
2017-12-15 3:47 ` [PATCH 2/6] btrfs: no need to check for btrfs_fs_devices::seeding Anand Jain
2017-12-15 3:47 ` [PATCH 3/6] btrfs: make btrfs_free_stale_device() to iterate all stales Anand Jain
2017-12-15 15:06 ` Nikolay Borisov
2017-12-16 2:13 ` Anand Jain
2017-12-16 6:54 ` Nikolay Borisov
2017-12-15 3:47 ` [PATCH 4/6] btrfs: make btrfs_free_stale_device() argument optional Anand Jain
2017-12-15 15:07 ` Nikolay Borisov
2017-12-16 2:14 ` Anand Jain
2017-12-16 6:55 ` Nikolay Borisov [this message]
2017-12-15 3:47 ` [PATCH 5/6] btrfs: make btrfs_free_stale_device() to match the path Anand Jain
2017-12-15 3:47 ` [PATCH 6/6] btrfs: cleanup to make btrfs_free_stale_device() readable Anand Jain
2017-12-15 11:29 ` [PATCH v2 1/6] " Anand Jain
2017-12-15 13:07 ` [PATCH v2 6/6] " Anand Jain
2017-12-16 2:48 ` [PATCH v3 " Anand Jain
-- strict thread matches above, loose matches on Subject: below --
2018-01-09 14:13 [PATCH RESEND v3 0/6] preparatory work to add device forget Anand Jain
2018-01-09 14:13 ` [PATCH 4/6] btrfs: make btrfs_free_stale_device() argument optional Anand Jain
2018-01-09 16:22 ` Josef Bacik
2018-01-10 5:15 [PATCH v4 0/6] preparatory work to add device forget Anand Jain
2018-01-10 5:15 ` [PATCH 4/6] btrfs: make btrfs_free_stale_device() argument optional Anand Jain
2018-01-10 15:50 ` Josef Bacik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d6ac16e5-2f11-ae8f-b5cd-97ef71e94b2d@suse.com \
--to=nborisov@suse.com \
--cc=anand.jain@oracle.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).