Hi Chris,
On 25 July 2010 15:42, Josef Bacik <josef@xxxxxxxxxx> wrote:
> On Sat, Jul 24, 2010 at 12:01:59AM +0100, Daniel J Blueman wrote:
>> Hi Chris,
>>
>> This fixes some issues relating to direct I/O submission, however a
>> further patch will be needed to handle the case where allocation of
>> 'dip' fails, which is always dereferenced when finding the ordered
>> extent.
>>
>
> Hi,
>
> There's an easier way to do this. This patch should fix the problem,
>
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxx>
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 3232945..7259ef9 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -5815,7 +5815,7 @@ free_ordered:
> if (write) {
> struct btrfs_ordered_extent *ordered;
> ordered = btrfs_lookup_ordered_extent(inode,
> - dip->logical_offset);
> + file_offset);
> if (!test_bit(BTRFS_ORDERED_PREALLOC, &ordered->flags) &&
> !test_bit(BTRFS_ORDERED_NOCOW, &ordered->flags))
> btrfs_free_reserved_extent(root, ordered->start,
>
Good move!
With your patch applied, mine (now not priority) then becomes:
Fix leak of 'dip' on error path and double assignment.
Signed-off-by: Daniel J Blueman <daniel.blueman@xxxxxxxxx>
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 1bff92a..bd7f940 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5652,15 +5652,15 @@ static void btrfs_submit_direct(int rw, struct
bio *bio, struct inode *inode,
ret = -ENOMEM;
goto free_ordered;
}
- dip->csums = NULL;
if (!skip_sum) {
dip->csums = kmalloc(sizeof(u32) * bio->bi_vcnt, GFP_NOFS);
if (!dip->csums) {
ret = -ENOMEM;
- goto free_ordered;
+ goto out_err;
}
- }
+ } else
+ dip->csums = NULL;
dip->private = bio->bi_private;
dip->inode = inode;
I didn't see Josef and my patches at
http://git.kernel.org/?p=linux/kernel/git/mason/btrfs-unstable.git
yet.
They still appear relevant; let me know if you'd like it rediffed
against your tree.
Thanks,
Daniel
--
Daniel J Blueman
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
