Re: New distributed storage release.
From: Evgeniy Polyakov
Date: Mon Sep 08 2008 - 15:32:44 EST
On Mon, Sep 08, 2008 at 08:19:23PM +0200, Sven Wegener (sven.wegener@xxxxxxxxxxx) wrote:
> > > drivers/block/dst/trans.c:160: warning: format '%llu' expects type 'long long unsigned int', but argument 4 has type 'sector_t'
> >
> > Yup, sector_t is diffrent depending on arch and config options (u64 vs unsigned long),
> > so it is not possible to represent it correctly without dereferencing
> > to another type.
>
> Yup, cast it. That are a lot of warnings and best to avoid them.
Yes, warnings have to be removed.
> > > uhm, menuconfig, and then just the debug option there seems wrong.
> >
> > In case of extended functionality there will be no need to change config
> > options, now it looks like single string in the higher layer config.
>
> If there is coming more funtionality, ok, but currently it's irritating to
> have a submenu and then just a single debug option in there.
Nothing is actually planned, I do not care which config option will be
used for that, just do not want to get a new claims on that it should be
changed.
> Yeah, the dprintk()s are ok, but I was after the printk()s that follow.
> They are unconditional and will always print the key.
Ugh, you are right, I missed that hunk.
> > > sizeof(*req) is preferred, likewise for other sizeof() uses in the code.
> >
> > No, I do belive that grepping over |struct ablkcipher_request| when
> > something is about to be changed is more convenient, than searching for
> > the structure name and then object name itself.
> > It is matter of a taste though.
>
> Yeah, but in nearly all cases you don't need to change anything in that
> line, when you use sizeof(*ptr), because it always gets you the correct
> size you want. There are cases we could argue about, because you might
> want to do a limited memset() or memcpy() with a different size, but for
> most cases like memset() to zero or kmalloc, sizeof(*ptr) makes it clear
> that you want to clear out the whole structure or you want enough memory
> to store the type of ptr in it. Don't know how strict the policy is, but
> it's part of CodingStyle.
Fortunately I missed that flamewar about coding style.
> > I just want to shut up the compiler, since failing to register
> > informational attribute is not critical. But it could also be
> > used to fall the initialization.
>
> For just shutting up the compiler, use the void cast. I think removing a
> failed attribute is ok.
If I will ut (void) casting there, I'm pretty sure I will be blamed
not to check th return values, now I check it :)
> > 36 bytes iirc - not that small for stack allocation I think.
>
> In other cases you allocate u8 iv[32] on the stack, so I guess this point
> is void. And I think 36 bytes isn't that much and it looks like you're
> called from user space, so I don't think you have a deep callchain here.
That's a fast path, so I optimized a bit, but I agree, iv initializaion
could be also moved to the preallocated area.
I think connector message can be allocated on stack, since I know the
whole callchain in that path.
I will updated the patch.
--
Evgeniy Polyakov
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/