On Wed, 5 Sep 2012 09:32:16 -0500 Brassow Jonathan <jbrassow@xxxxxxxxxx> wrote: > > On Sep 4, 2012, at 8:44 PM, NeilBrown wrote: > > > On Thu, 23 Aug 2012 10:31:54 -0500 Jonathan Brassow <jbrassow@xxxxxxxxxx> > > wrote: > > > >> Neil, > >> > >> There are two ways I see to fix the problem of the "sync" table argument > >> not causing the array to re-sync. We could set mddev->events to > >> UINT_MAX and force an mddev/bitmap event mismatch - causing the bitmap > >> code to set BITMAP_STALE itself. Otherwise, we can do as I have below > >> and set BITMAP_STALE ourselves. The real difference is whether this is > >> done before or after md_run(). The first solution can be done before > >> md_run(); the second cannot because the bitmap is allocated during > >> md_run(). Either solution works and correctly sets the bitmap to stale > >> before bitmap_load() is called. So, rather than having to think about > >> issues that come with setting events to UINT_MAX, I chose to set the > >> bitmap stale after md_run(). > > > > I'm not at all sure this is the right way to look at the issue. > > > > The only reason that we have 'sync' and 'nosync' table args is because > > originally we weren't storing any metadata on disk. The 'in-sync?' state of > > the array is important metadata, so it needed to be given to md via a table > > option. > > > > If you have a bitmap, then you are storing this information in metadata, so > > including it as a table option is redundant and - as you noticed - ignored. > > > > If you want to assert that the data in the bitmap is incorrect, then I think > > the best approach is to change it so that it is correct. > > i.e. if you want to assert that an array is not in-sync and should be fully > > 'synced', then the right thing to do is to write lots of '1's to the bitmap > > before trying to assemble the array. > > > > However I'm not violently against the patch - though I would be against > > fiddling with event_count. It does seem unpleasantly asymmetric though. > > The 'sync' option says 'assume all bits in the bitmap are set', but the > > 'nosync' option doesn't say 'assume all bits in the bitmap are clear'. > > > > Is simply writing to the bitmap when you want to force some behaviour an > > option, or do you really really want "sync" to mean "please ignore the > > bitmap"? (or maybe an "ignorebitmap" option is what we want??). > > For me, the 'sync' and 'nosync' args are not just a vestige of when we didn't have metadata; but also a form of continuity with dm-raid1. The 'sync' option has - for device-mapper - always been a way to reset the bitmap and force a resync. With dm-raid1, it is dm-log.c which interprets the 'sync' and 'nosync' flags. The meaning seems to be: - 'sync' - ignore the log, force the array to be synced (i.e. set all regions to 'dirty') - 'nosync' - ignore the log, don't sync the array (i.e. set all regions to 'clean') - neither of those - use the log to determine whether to sync or not. If you want to make the flags to dm-raid mean exactly the same as that (i.e. if either sync or nosync is present, we ignore the metadata) then I would be happy with that. I don't think that is quite what your patch does though - it only does half of it. NeilBrown
Attachment:
signature.asc
Description: PGP signature
