On Mon, Nov 07, 2016 at 05:58:41PM +0100, David Sterba wrote: > On Fri, Nov 04, 2016 at 08:26:54AM +0100, Adam Borowski wrote: > > They're not even documented anywhere, letting users with no recourse but > > to RTFS. It's no big burden to output the bitfield as words. > > Ok. Below are some comments (and style nitpicks). [...] > > + char flags_str[128]; > > This could be a problem. The relocation can trigger deep call chains > when doing IO, that need stack. I'd rather avoid the static buffer, > please switch it to kmalloc. Are the chains callers or callees of relocation? I believe exclusively the latter, as balance is a long process that wouldn't make sense inside a deep chain -- am I right? In that case, moving the printk to the display function would localize the buffer, avoiding all complications of an alloc. (v2 does alloc) > As this is not a critical allocation, the fallback would be to print just > the raw value as now. If the kernel is that low on memory, is it reasonable to print an unimportant message? As I see, lots of btrfs code would just fall over and die horribly, with BUG_ON or aborting; perhaps skipping the message would be safer? (v2 has a fallback message) > Also, please use snprintf. This would need extra variable to track > number of already printed chars, but should be easy M'kay. Even with all flags on (most are mutually exclusive) we won't get anywhere close to 128 characters, but paranoia can't hurt. Meow! -- A MAP07 (Dead Simple) raspberry tincture recipe: 0.5l 95% alcohol, 1kg raspberries, 0.4kg sugar; put into a big jar for 1 month. Filter out and throw away the fruits (can dump them into a cake, etc), let the drink age at least 3-6 months. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
