Hi Neil,


On Fri, May 22, 2009 at 2:28 PM, NeilBrown [off-list ref] wrote:

On Fri, May 22, 2009 6:36 pm, Andre Noll wrote:

quoted

On 13:48, SandeepKsinha wrote:

quoted

Signed-off-by: Sandeep K Sinha <redacted>
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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html