linux-kernel - Re: [PATCH 0/3] zram: Replace pr_* with dev

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] [day] [month] [year] [list]

Message-ID: <20150810012527.GC645@swordfish>
Date:	Mon, 10 Aug 2015 10:25:27 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:	Minchan Kim <minchan@...nel.org>
Cc:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
	Salah Triki <salah.triki@....org>, ngupta@...are.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3] zram: Replace pr_* with dev_*

On (08/07/15 23:58), Minchan Kim wrote:
[..]
> > note the prefix 'zram:' became 'block zram0:'
> 
> It would be better because it's more clear if we can make several
> blocks.

in that case I'd rather prefer to add zram%d to some of the existing
messages, than mix pr_* with dev_*. besides, 'block zram0:' is a bit
too long, a short 'zram0:' looks better.

well... we already return -errno from every path that does interact
with user space, which is (I think) good enough; adding device_ids
to those 'error return paths' may be OK.

for example. suppose zram is a swap device and we have !handle case

        handle = zs_malloc(meta->mem_pool, clen);
        if (!handle) {
                pr_info("Error allocating memory for compressed page: %u, size=%zu\n",
                        index, clen);
                ret = -ENOMEM;
                goto out;
        }

if zs_malloc() failed, then the system is in such big troubles, that
device_id in the message is the last thing anyone will look at.

so I'm skeptical. the benefit is sort of minimal.

[..]
> > 
> > where %d is a supplied max_comp_stream value
> > 
> > From: "Can't change algorithm for initialized device\n"
> > To: "Can't change algorithm to %s for initialized device\n"
> 
> It's okay but hope to have prefix "block zram0"

it's not ok. I haven't tested it, but I think that the error message
is broken. the supplied string most likely will contain trailing
\n (I think people usually use echo, not echo -n), so it will be:
	Can't change algorithm to XXX
	 for initialized device

... haven't tested.

BUT why would compression algorithm name even matter here?? the error
is 'you attempted to configure _an already configured device_', not
'you attempted to configure an already configured device _with XXX_'.
that XXX part is not relevant here, 'already configured device' is.

	-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/