mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Guillaume Thouvenin <guillaume.thouvenin@bull.net>
To: Jay Lan <jlan@sgi.com>
Cc: Guillaume Thouvenin <guillaume.thouvenin@bull.net>,
	linux-kernel@vger.kernel.org,
	Christoph Hellwig <hch@infradead.org>,
	Andrew Morton <akpm@osdl.org>, John Hesterberg <jh@sgi.com>
Subject: Re: [patch 2.6.8.1] BSD accounting: update chars transferred value
Date: Mon, 13 Sep 2004 08:34:44 +0200	[thread overview]
Message-ID: <20040913063444.GA17636@frec.bull.fr> (raw)
In-Reply-To: <41423480.8090104@sgi.com>

On Fri, Sep 10, 2004 at 04:10:56PM -0700, Jay Lan wrote:
> This patch is a subset of csa_io with your patch deals with character
> IO only.

Yes you are right. This patch only deals with character IO because I 
don't know yet how to get information for blocks IO. As I said my goal 
is to provide a good solution for accounting. BSD-accounting is already 
in the kernel and CSA provide more metrics so I think it could be good 
to add some CSA accounting values in the BSD-accounting. 

> I can see that merge csa_io's change at vfs_writev() and vfs_readv()
> into your change at do_readv_writev(). However, the code change is
> not really common code in that a) the operation type is different and
> 2) the fields to add to are different, so you end up doing extra check 
> of file operation type (READ vs WRITE). I would be happy if either
> your patch or mine is accepted, but i think it does extra work to put
> the changes into the common routine (ie do_readv_writev).

As you notice, vfs_writev() and vfs_readv() both call do_readv_writev()
and as fields to add are different I added a test on the operation type.
I though that it was interesting to put a changes in the common routine
but you are right that it has a cost (the file operation check). As the 
changes can be done in vfs_readv() and vfs_writev() instead of one single 
routine (do_readv_writev()) I though this choice was good but if the
extra cost is a problem I agree with your solution. Thank you to point
this out.

> Shall we combine your patch and SGI's csa_io patch?

IMHO, it could be very interesting to combine your patch and mine.
BSD-accounting is doing per-process accounting and CSA also doing
per-process accounting even if the goal is a per-job accounting. Thus, I
think that it can be good to combine both. It isn't necessary to have
two different accounting systems in the kernel. 

Is it difficult for you to add what you are doing with CSA in the
BSD-accounting file? Maybe the solution is to remove BSD-accounting in
favor of CSA accounting? Personally, I don't care if we keep
BSD-accounting or if we remove it to add CSA accounting.

Best,
Guillaume

  reply	other threads:[~2004-09-13  6:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-09-08 11:29 Guillaume Thouvenin
2004-09-10 23:10 ` Jay Lan
2004-09-13  6:34   ` Guillaume Thouvenin [this message]
2004-09-14  0:00     ` Jay Lan
  -- strict thread matches above, loose matches on Subject: below --
2004-09-08  9:06 Guillaume Thouvenin
2004-09-08  9:17 ` Christoph Hellwig
2004-09-08 11:02   ` Guillaume Thouvenin

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=20040913063444.GA17636@frec.bull.fr \
    --to=guillaume.thouvenin@bull.net \
    --cc=akpm@osdl.org \
    --cc=hch@infradead.org \
    --cc=jh@sgi.com \
    --cc=jlan@sgi.com \
    --cc=linux-kernel@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

all inboxes | Powered by JetHome®