On Wed, Jun 01, 2016 at 09:23:57AM +0800, luke wrote:
>
>
> At 06/01/2016 12:15 AM, David Sterba wrote:
> > On Tue, May 31, 2016 at 11:08:39AM +0800, luke wrote:
> >>>> +};
> >>>> +
> >>>> +/* dynamically allocate and initialize a ref_root */
> >>>> +static struct ref_root *ref_root_alloc(gfp_t gfp_mask)
> >>>> +{
> >>>> + struct ref_root *ref_tree;
> >>>> +
> >>>> + ref_tree = kmalloc(sizeof(*ref_tree), gfp_mask);
> >>> Drop the gfp_mask and make it GFP_KERNEL
> >> OK, I'll drop the gfp_mask, but can you tell me why should use
> >> GFP_KERNEL instead of GFP_NOFS?
> > Because there's no need to narrow the allocation constraints. GFP_NOFS
> > is necessary when the caller is on a critical path that must not recurse
> > back to the filesystem through the allocation (ie. if the allocator
> > decides to free some memory and tries tro write dirty data). FIEMAP is
> > called from an ioctl.
> OK, I'll update this patch with GFP_KERNEL.
> >
> >>>> disko = em->block_start + offset_in_extent;
> >>>>
> >>>> /*
> >>>> + * We need a trans handle to get delayed refs
> >>>> + */
> >>>> + trans = btrfs_join_transaction(root);
> >>> What are the implications of join/end transaction here? It's just
> >>> fiemap, I would not expect messing with transaction here.
> >> This transaction is used to handle delayed_refs, if trans = NULL, we
> >> have to ignore the delayed_refs.
> > Yes that's what the comment says as well, but I still don't understand
> > why, so I need somebody else to review that part.
> If not checking delayed refs, shared_flag will not be set for reflink
> until transaction committed.
> Test case to test this misbehavior is already submitted(generic/353).
>
> The simplest test procedure would be like the following:
>
> # xfs_io -f -c "pwrite 0 128K" test
> # cp --reflink test test2
> # xfs_io -c "fiemap" test2
>
> And normally, the SHARED flag should be set, but since the trans isn't
> committed yet, if not checking delayed refs, we can't determine whether
> it is shared by others.
Ok, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
