linux-kernel - Re: [PATCH] btrfs: add lockdep_assert_held to need_preemptive

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220302193042.GV12643@twin.jikos.cz>
Date:   Wed, 2 Mar 2022 20:30:42 +0100
From:   David Sterba <dsterba@...e.cz>
To:     Niels Dossche <dossche.niels@...il.com>
Cc:     linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
        David Sterba <dsterba@...e.com>,
        Johannes Thumshirn <johannes.thumshirn@....com>
Subject: Re: [PATCH] btrfs: add lockdep_assert_held to need_preemptive_reclaim

On Mon, Feb 28, 2022 at 11:52:16PM +0100, Niels Dossche wrote:
> In a previous patch I extended the locking for member accesses of
> space_info.

A reference to another patch would be by a subject or a specific commit
id (not applicable in this case) or you can write it without any
reference if the change is standalone. The changelog should describe the
reason for the change, user visible effects, what can go wrong etc.

> It was then suggested to also add a lockdep assertion for
> space_info->lock to need_preemptive_reclaim.
> 
> Suggested-by: Johannes Thumshirn <johannes.thumshirn@....com>
> Signed-off-by: Niels Dossche <dossche.niels@...il.com>
> ---
>  fs/btrfs/space-info.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index 294242c194d8..5464bd168d5b 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -734,9 +734,13 @@ static bool need_preemptive_reclaim(struct btrfs_fs_info *fs_info,
>  {
>  	u64 global_rsv_size = fs_info->global_block_rsv.reserved;
>  	u64 ordered, delalloc;
> -	u64 thresh = div_factor_fine(space_info->total_bytes, 90);
> +	u64 thresh;
>  	u64 used;
>  
> +	lockdep_assert_held(&space_info->lock);
> +
> +	thresh = div_factor_fine(space_info->total_bytes, 90);

I'm not sure this is necessary, as this is not locking where the
initialization would have to be inside. The lockdep assertion is just a
warning, so it does not matter where the intialization is done, I'd
prefer to keep it as is.