Date: Mon, 10 Aug 2015 10:25:27 +0900
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
        Salah Triki <salah.triki@acm.org>, ngupta@vflare.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/3] zram: Replace pr_* with dev_*
Message-ID: <20150810012527.GC645@swordfish>
References: <cover.1438899649.git.salah.triki@acm.org>
 <20150807000520.GA1891@swordfish>
 <20150807060559.GA7716@bgram>
 <20150807063756.GH1891@swordfish>
 <20150807145856.GB32614@blaptop>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150807145856.GB32614@blaptop>
User-Agent: Mutt/1.5.23+102 (2ca89bed6448) (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2104
Lines: 61

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/