From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 2/3] xfs: use rhashtable to track buffer cache
Date: Wed, 7 Dec 2016 17:32:13 +1100 [thread overview]
Message-ID: <20161207063213.GI4326@dastard> (raw)
In-Reply-To: <20161205170411.GA14091@infradead.org>
On Mon, Dec 05, 2016 at 09:04:11AM -0800, Christoph Hellwig wrote:
> > +static int
> > +_xfs_buf_cmp(
> > + struct rhashtable_compare_arg *arg,
> > + const void *obj)
> > +{
> > + const struct xfs_buf_map *map = arg->key;
> > + const struct xfs_buf *bp = obj;
>
> How about aligning the parameter / variable names here like we
> normally do in XFS?
Sorry, forgot to do that when I updated the patch.
> > +static const struct rhashtable_params xfs_buf_hash_params = {
> > + .min_size = 32, /* empty/unused AGs have minimal footprint */
> > + .nelem_hint = 16,
> > + .key_len = sizeof(xfs_daddr_t),
> > + .key_offset = offsetof(struct xfs_buf, b_bn),
> > + .head_offset = offsetof(struct xfs_buf, b_rhash_head),
> > + .automatic_shrinking = true,
> > + .obj_cmpfn = _xfs_buf_cmp,
>
> Some tab alignments before the equal signs here?
>
> Also please name the compare function so that it fits the field name,
> as that makes grepping so much easier..
>
> .obj_cmpfn = xfs_buf_obj_cmpfn,
Will do.
> Otherwise this looks fine to me and seems to survive testing so far:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
Thanks.
-Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2016-12-07 6:38 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-01 10:30 [PATCH 0/3] xfs: miscellaneous improvements for 4.10 Dave Chinner
2016-12-01 10:30 ` [PATCH 1/3] xfs: make xfs btree stats less huge Dave Chinner
2016-12-02 13:20 ` Christoph Hellwig
2016-12-05 1:14 ` Dave Chinner
2016-12-05 12:48 ` Christoph Hellwig
2016-12-01 10:30 ` [PATCH 2/3] xfs: use rhashtable to track buffer cache Dave Chinner
2016-12-05 17:04 ` Christoph Hellwig
2016-12-07 6:32 ` Dave Chinner [this message]
2016-12-01 10:30 ` [PATCH 3/3] xfs: optimise CRC updates Dave Chinner
2016-12-02 13:22 ` Christoph Hellwig
2016-12-04 22:12 ` Dave Chinner
2016-12-05 12:48 ` Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161207063213.GI4326@dastard \
--to=david@fromorbit.com \
--cc=hch@infradead.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).