Hi Neil,
On Fri, May 22, 2009 at 2:28 PM, NeilBrown <neilb@xxxxxxx> wrote:
> On Fri, May 22, 2009 6:36 pm, Andre Noll wrote:
>> On 13:48, SandeepKsinha wrote:
>>> Signed-off-by: Sandeep K Sinha <sandeepksinha@xxxxxxxxx>
>>> Date: Fri May 22 13:46:56 2009 +0530
>>>
>>> Protecting mddev with barriers to avoid races.
>>>
>>> diff --git a/drivers/md/linear.c b/drivers/md/linear.c
>>> index 9ad6ec4..e2dbda8 100644
>>> --- a/drivers/md/linear.c
>>> +++ b/drivers/md/linear.c
>>> @@ -28,10 +28,12 @@
>>> static inline dev_info_t *which_dev(mddev_t *mddev, sector_t sector)
>>> {
>>> int lo, mid, hi;
>>> - linear_conf_t *conf = mddev_to_conf(mddev);
>>> + linear_conf_t *conf;
>>>
>>> lo = 0;
>>> hi = mddev->raid_disks - 1;
>>> + barrier();
>>> + conf = mddev_to_conf(mddev);
>>
>> Are you sure that a compiler barrier is sufficient here? Maybe we
>> need smp_mb(), but I'm not familar with all the different memory
>> barrier types that exist in the kernel.
>
> I think we just want a read barrier here:
> smb_rmb();
> because it is a barrier between reading mddev->raid_disk and
> reading mddev->private.
>
> In linear_add, we want a write barrier
> smb_wmb();
> before setting mdev->raid_disks, and I think we also want one
> before setting mddev->private to make sure that all the assignments
> to newconf->whatever are stable.
Do we really need one before setting mddev->private? Will newconf be
used by anyone before its assigned to mddev->private?
No. Then what makes you put a write barrier before it?
> NeilBrown
>
>
>
--
Regards,
Sandeep.
“To learn is to change. Education is a process that changes the learner.”
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
