On 09/09/2017 02:00 PM, Linus Torvalds wrote: > On Sat, Sep 9, 2017 at 12:54 PM, Linus Torvalds > <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: >> >> Other than that, we match. > > Oh, and I almost cried when I saw this nasty thing: > > ilog2(SZ_4K) - 9 > > in that nvme code, but I left it alone. > > Why the hell people think "SZ_4K" is somehow more legible than 4096, I > have no idea. And depending on our ilog2() just doing the right thing > at build time, instead of just saying "12" with a comment, is just > odd. Beats me, I hate all of the SZ_ defines. But once they exist, they grow users... > It's not like "ilog2(SZ_4K)" is a particularly good comment. > > That code should probably just make it clear that 4096 is the NVME > page size, and document it as such, rather than picking a random name > for a random constant. > > So something like > > #define NVME_PAGE_SIZE 4096 > #define NVME_PAGE_SHIFT 12 > > and then using *those* would actually have documented something, > rather than these crazy odd random expressions. Totally agree. -- Jens Axboe