lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAEH94LipqaUaRNZv-X_=eZpdrMCAL3DhyxLSOeXLSmdjbNNLpQ@mail.gmail.com> Date: Thu, 10 Jan 2013 14:24:50 +0800 From: Zhi Yong Wu <zwu.kernel@...il.com> To: dsterba@...e.cz Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, viro@...iv.linux.org.uk, david@...morbit.com, darrick.wong@...cle.com, andi@...stfloor.org, hch@...radead.org, linuxram@...ux.vnet.ibm.com, wuzhy@...ux.vnet.ibm.com Subject: Re: [PATCH RESEND v1 01/16] vfs: introduce some data structures On Thu, Jan 10, 2013 at 8:48 AM, David Sterba <dsterba@...e.cz> wrote: > On Thu, Dec 20, 2012 at 10:43:20PM +0800, zwu.kernel@...il.com wrote: >> --- /dev/null >> +++ b/fs/hot_tracking.c >> @@ -0,0 +1,109 @@ >> +/* >> + * fs/hot_tracking.c > > From what I've undrestood the file name written here is not wanted, so > please drop it (and from .h too) Done. > >> + * >> + * Copyright (C) 2012 IBM Corp. All rights reserved. >> + * Written by Zhi Yong Wu <wuzhy@...ux.vnet.ibm.com> >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public >> + * License v2 as published by the Free Software Foundation. > > A short description of the hot tracking feature or pointer to the > Documentation/ file would be nice here. ok, Done > >> + */ >> + >> +#include <linux/list.h> >> +#include <linux/err.h> >> +#include <linux/slab.h> >> +#include <linux/module.h> >> +#include <linux/spinlock.h> >> +#include <linux/hardirq.h> >> +#include <linux/fs.h> >> +#include <linux/blkdev.h> >> +#include <linux/types.h> >> +#include <linux/limits.h> >> +#include "hot_tracking.h" >> + >> +/* kmem_cache pointers for slab caches */ > > This comment seems useless to me, I does not help understanding the code, just > says the same what reads in C. There are more such redundant comments in the > series, but I'm not going point to all of them right now. Removed. > >> +static struct kmem_cache *hot_inode_item_cachep __read_mostly; >> +static struct kmem_cache *hot_range_item_cachep __read_mostly; >> + > >> --- /dev/null >> +++ b/include/linux/hot_tracking.h >> +/* The common info for both following structures */ >> +struct hot_comm_item { >> + struct rb_node rb_node; /* rbtree index */ >> + struct hot_freq_data hot_freq_data; /* frequency data */ >> + spinlock_t lock; /* protects object data */ >> + struct kref refs; /* prevents kfree */ >> +}; >> + >> +/* An item representing an inode and its access frequency */ >> +struct hot_inode_item { >> + struct hot_comm_item hot_inode; /* node in hot_inode_tree */ >> + struct hot_rb_tree hot_range_tree; /* tree of ranges */ >> + spinlock_t lock; /* protect range tree */ >> + struct hot_rb_tree *hot_inode_tree; >> + u64 i_ino; /* inode number from inode */ >> +}; > > Please align the comments to something like this (or drop them if they seem > redundant): Done > > /* The common info for both following structures */ > struct hot_comm_item { > struct rb_node rb_node; /* rbtree index */ > struct hot_freq_data hot_freq_data; /* frequency data */ > spinlock_t lock; /* protects object data */ > struct kref refs; /* prevents kfree */ > struct list_head n_list; /* list node index */ > }; > > /* An item representing an inode and its access frequency */ > struct hot_inode_item { > struct hot_comm_item hot_inode; /* node in hot_inode_tree */ > struct hot_rb_tree hot_range_tree; /* tree of ranges */ > spinlock_t lock; /* protect range tree */ > struct hot_rb_tree *hot_inode_tree; > u64 i_ino; /* inode number from inode */ > }; > >> +extern void __init hot_cache_init(void); > > this belongs to the private include fs/hot_tracking.h (because this is called > only once by vfs init and not by filesystems), there's > hot_track_init(superblock) for that purpose introduced later. Done, Move it to fs/hot_tracking.h > > > david -- Regards, Zhi Yong Wu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists