On Wed, May 18, 2016 at 5:00 AM, Namhyung Kim <namhy...@kernel.org> wrote:
> The commit f0e2efcfd2717 ("pstore: do not use message compression
> without lock") added a check to 'is_locked' to avoid breakage in
> concurrent accesses. But it has a side-effect of disabling compression
> on normal path since 'is_locked' variable is not set. As normal path
> always takes the lock, it should be initialized to 1.
>
> This also makes the unlock code a bit simpler.
>
> Signed-off-by: Namhyung Kim <namhy...@kernel.org>
> ---
> fs/pstore/platform.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 588461bb2dd4..04a0164a2066 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -284,7 +284,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
> u64 id;
> unsigned int part = 1;
> unsigned long flags = 0;
> - int is_locked = 0;
> + int is_locked = 1;
> int ret;
>
> why = get_reason_str(reason);
> @@ -350,10 +350,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
> total += total_len;
> part++;
> }
> - if (pstore_cannot_block_path(reason)) {
> - if (is_locked)
> - spin_unlock_irqrestore(&psinfo->buf_lock, flags);
> - } else
> + if (is_locked)
> spin_unlock_irqrestore(&psinfo->buf_lock, flags);
> }
This makes sense, but I think I'd prefer that is_locked get set in the
"else", since that makes it a little easier to read, IMO. I'll adjust
it and add it to the tree.
Thanks!
-Kees
--
Kees Cook
Chrome OS & Brillo Security
