On Thu, Aug 23, 2012 at 06:24:02PM +0800, Liu Bo wrote: > > Hum. I added this because I wanted to avoid allocations > PAGE_SIZE. We're > > doing > > kmalloc GFP_NOFS with whatever one enters as size, I'm not sure that's a > > good > > idea without any sanitizing. > > Yeah, I agree. > > So we do need to make some sanity checks, according to my tests, > we need about 30k to resolve a file shared by 4000 snapshots. > > What about 32k as a upside limit?
64k? As it's an upper limit, let's make it a bit higher than what one can reach normally. > > Second, we should probably add a fall back option to vmalloc, in case > > kmalloc > > fails? Or should we even go for vmalloc directly, what do you think? > > Given loi->size is not reliable, going for vmalloc for an ioctl is reasonable. Yeah, vmalloc is ok here, we can safely afford to return ENOMEM. AFAICS, we could possibly reuse the userspace buffer, iff build_ino_list uses copy_to_user instead of directly writing to the data buffer. This would eliminate the kernel-side ENOMEM completely at the cost of function call overhead and the access_ok checks. And compared to the simplicity of just vmalloc with rare occurence of ENOMEM, I don't think it's worth the work. david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
