On Tue, 2014-04-08 at 16:06 +0100, Anton Altaparmakov wrote: > Hi Hin-Tak, > > That now looks good. > > I agree with you that doing a major cleanup of how attribute names are > handled is a separate patch. As you say that is much more work and it > is not as trivial as this. I think doing the attribute handling > properly requires reworking how it all fits together because it is > imposing the name length checks at the wrong level - they need to be > done after conversion to UTF-16 not whilst they are still in the > current NLS (except of course you want to check that they are not too > big to fit in the buffer though it is not at all clear to me why the > argument is being copied into a buffer in the first place at least in > one of the places I looked at - seems a waste of CPU time to me). > Yes, it is possible to split the work on several patches. I agree with it. But I suppose that these patches needs to be joined in one patchset. Because reported issue can be solved by fix as for file names as for xattr names. Of course, it is possible to fix the issue with file names, firstly, and then the issue with xattr names. But it doesn't make sense to mix fixes for file names and for xattr names in one patch. Because partial fix in hfsplus_listxattr() doesn't fix the issue for xattr names. > Also, I don't think it is needed to set up a kmem cache for this - > kmalloc is perfect - as at any one time there will be a vanishingly > small number of these buffers allocated at any one time. After all, > how many concurrent readdir() calls are there going to be on a volume > at any one time? It is not like the struct inode or other long lived > memory structures where you end up with thousands of them in memory at > any one time... Yes, kmalloc can be enough for readdir() case. But I suppose that it needs to use kmem cache for the case of xattr names. It will be more efficient solution. Thanks, Vyacheslav Dubeyko. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
