Hi Mitch,
having this patch on the list is a good idea. I've two remarks, just in
case it will be included sometimes:
On 09.02.2012 22:38, Mitch Harder wrote:
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index fe4cd0f..31b717f 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -656,8 +656,18 @@ static int btrfs_delayed_inode_reserve_metadata(
> * EAGAIN to make us stop the transaction we have, so return
> * ENOSPC instead so that btrfs_dirty_inode knows what to do.
> */
> +#ifdef BTRFS_DEBUG_ENOSPC
> + if (unlikely(ret == -EAGAIN)) {
> + ret = -ENOSPC;
> + if (printk_ratelimit())
>From linux/printk.h:
/*
* Please don't use printk_ratelimit(), because it shares ratelimiting state
* with all other unrelated printk_ratelimit() callsites. Instead use
* printk_ratelimited() or plain old __ratelimit().
*/
printk_ratelimited() seems the right choice, here.
> + printk(KERN_WARNING
> + "btrfs: ENOSPC set in "
> +
> "btrfs_delayed_inode_reserve_metadata\n");
> + }
> +#else
> if (ret == -EAGAIN)
> ret = -ENOSPC;
> +#endif
I don't like the #if placements throughout the patch. Copying all the
conditions is error prone (especially when changes are made later on).
I'd rather leave all the conditions as they stand (possibly adding the
unlikely() macro if you wish). Same for the following return statement
or assignment. Simply put printk_ratelimited() into the #if ... #endif.
-Jan
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
