On Monday 21 July 2014 at 00:49:14, Edward Shishkin wrote: > > On 07/20/2014 11:04 PM, Edward Shishkin wrote: > > > > On 07/20/2014 02:33 PM, Edward Shishkin wrote: > >> > >> On 07/20/2014 12:06 PM, Ivan Shapovalov wrote: > >>>> 20 июля 2014 г., в 1:20, Edward Shishkin > >>>> <edward.shishkin@xxxxxxxxx> написал(а): > >>>> > >>>> > >>>>> On 07/14/2014 03:56 AM, Edward Shishkin wrote: > >>>>> > >>>>>> On 07/13/2014 09:18 PM, Ivan Shapovalov wrote: > >>>> [...] > >>>>>> Nothing seems to call at least check_free_blocks(begin, end)... > >>>>> > >>>>> Oh, bad... I thought all this time that extents of the delete sets > >>>>> are still dirty > >>>>> in the working bitmap at the moment of discard. > >>>>> Hmm, I definitely don't want to check the whole extents for > >>>>> discard... > >>>> > >>>> False alarm, actually.. > >>>> > >>>> Nothing is wrong if someone makes them dirty before issuing our > >>>> discard > >>>> requests: his changes will be committed in the next sessions of > >>>> possessing > >>>> the commit_mutex _after_ our discard. Everything is isolated.. For > >>>> the same > >>>> reason we don't care if someone allocate blocks in head and tail > >>>> paddings > >>>> before issuing our padded discard extents. > >>> I was just thinking about the very same possible race... > >>> > >>> But well, isn't the call to current_atom_complete_writes() placed > >>> before acquisition of commit_mutex? I was under impression of that > >>> relocate sets could be written while another atom is being committed > >>> under the mutex. > >> > >> take the commit_mutex before flush if discard_enabled && > >> should_check_heads. > > > > > > Actually, this is not so simple. E.g. I got a deadlock.. > > Another option is to make head/tail paddings dirty with the > > following update of the extent of the delete_set. > > One more option is to not support such discard params. > > > Actually what we are trying to implement is "precise discard". This is > rather complicated task. I looked at other file systems: nobody bother > with paddings, just stupidly pass a freed extent to blkdev_issue_discard(), > which cuts the head and the tail. I suggest to do the same. We have done > our best. Someone can not afford even to accumulate the extents. > > Edward. Let's do it ideally, after all, this is reiser4 ;) I also think that we can just mark paddings as allocated and add them to extent in question (so that they will be freed by reiser4_post_commit_hook). This looks simple enough. The block allocator interface, IIRC, allows us to tell it "allocate right here or return failure". /* I had no time to look into any code during this weekend. Tomorrow I'll hopefully repost the discard-before-dealloc patches and look into this.. */ -- Ivan Shapovalov / intelfx /
Attachment:
signature.asc
Description: This is a digitally signed message part.
