Re: reiser4: discard implementation, pass 2: allocation issues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wednesday 18 June 2014 at 03:41:04, Edward Shishkin wrote:	
> 
> On 06/17/2014 10:47 PM, Ivan Shapovalov wrote:
> > On Tuesday 17 June 2014 at 22:31:36, Ivan Shapovalov wrote:	
> >> On Tuesday 17 June 2014 at 12:29:53, Edward Shishkin wrote:	
> >>> [...]
> >>>
> >>> Yup.
> >>> So, if discard is on, we work with 2 lists (delete_set,
> >>> delete_set_for_wander).
> >>> If discard is off, we work with one blocknr set..
> >> Good. So I'll do roughly following for v5:
> >> - rename discard_set_* to block_list_* and split off these definitions
> >> - write a family of reiser4_atom_dset_*() (log_deferred, log_immediate,
> >>    apply_deferred, merge, init, destroy) which will encapsulate discard/nodiscard
> >>    check and operate on correct lists (blocknr_set vs block_list)
> >> - call reiser4_atom_dset_{init,destroy,merge}() from respective functions
> >> - call reiser4_atom_dset_log_{deferred,immediate}() from reiser4_dealloc_blocks()
> >> - call reiser4_atom_dset_apply_deferred() from reiser4_post_commit_hook()
> >> - directly manipulate the block lists from discard_atom(), checking that we
> >>    indeed have discard enabled
> >>
> >> Is this OK?
> > BTW, with txn_atoms there is a locking idiom involving E_REPEAT loops.
> >
> > Is it fine to implement a
> > current_atom_dset_log_...(...) // E_REPEAT loop inside
> > instead of
> > atom_dset_log_...(txn_atom* atom, ...)
> 
> IMHO it is not needed.
> 
> atom = get_current_atom_locked();
> atom_dset_{defer, immed}_add_extent(atom, ...); // <-- the loop is here
> spin_unlock_atom(atom);

IIUC, the point of the E_REPEAT loop is that memory allocations must be done
with atom spinlock released, and once released, the atom may "disappear", so
it is unsafe to use the same pointer again. Hence there is a loop of

void *data = NULL;

do {
    atom = get_current_atom_locked();
    ret = blocknr_set_add_extent(atom, &atom->delete_set, &data, ...);
} while (ret == -E_REPEAT);

where `data` is a piece of "state" saved across the two blocknr_set_add_extent()
calls. On first iteration of the loop, atom lock is released, memory is
allocated and pointer is stored in `data`, and -E_REPEAT is returned.
On second iteration of the loop, the previously allocated memory is accessed
and the atom is modified under its spinlock.

-- 
Ivan Shapovalov / intelfx /

Attachment: signature.asc
Description: This is a digitally signed message part.


[Index of Archives]     [Linux File System Development]     [Linux BTRFS]     [Linux NFS]     [Linux Filesystems]     [Ext4 Filesystem]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Resources]

  Powered by Linux