netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "David S. Miller" <davem@davemloft.net>
To: ak@suse.de
Cc: dada1@cosmosbay.com, bcrl@linux.intel.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] struct file cleanup : the very large file_ra_state is now allocated only on demand.
Date: Wed, 17 Aug 2005 19:43:15 -0700 (PDT)	[thread overview]
Message-ID: <20050817.194315.111196480.davem@davemloft.net> (raw)
In-Reply-To: <20050818010524.GW3996@wotan.suse.de>

From: Andi Kleen <ak@suse.de>
Date: Thu, 18 Aug 2005 03:05:25 +0200

> I would just set the ra pointer to a single global structure if the
> allocation fails. Then you can avoid all the other checks. It will
> slow down things and trash some state, but not fail and nobody
> should expect good performance after out of memory anyways. The only
> check still needed would be on freeing.

I would think twice about that due to repeatability concerns.  Yes, we
should care less when memory is so low, but if we can avoid this kind
of scenerio easily we should.

Having said that, I would like to recommend looking into a scheme
where the path leading to the filp allocation states whether the
read-ahead bits are needed or not.  This has two benefits:

1) Repeatability, and error signalling at the correct place
   should the memory allocation fail.

2) We can avoid the pointer dereference overhead.  The read-ahead
   state is always at (filp + 1).  Macro'ized or static inline
   function'ized interfaces for this access can make it look
   clean and perhaps even implement debugging of the case where
   we try to get at the read-ahead state for a non-read-ahead
   filp.

I do really think that would be a better approach.  A quick glance
shows that it should be easy to propagate the "need_read_ahead"
state, just by passing a boolean to get_unused_fd() via
sock_map_fd().

  reply	other threads:[~2005-08-18  2:43 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20050810164655.GB4162@linux.intel.com>
     [not found] ` <20050810.135306.79296985.davem@davemloft.net>
     [not found]   ` <20050810211737.GA21581@linux.intel.com>
     [not found]     ` <430391F1.9080900@cosmosbay.com>
     [not found]       ` <20050817211829.GK27628@wotan.suse.de>
     [not found]         ` <4303AEC4.3060901@cosmosbay.com>
     [not found]           ` <20050817215357.GU3996@wotan.suse.de>
2005-08-18  0:40             ` [PATCH] struct file cleanup : the very large file_ra_state is now allocated only on demand Eric Dumazet
2005-08-18  1:05               ` Andi Kleen
2005-08-18  2:43                 ` David S. Miller [this message]
2005-08-18  7:14                   ` Eric Dumazet
2005-08-18  7:18                     ` David S. Miller
2005-08-18  2:52                 ` Nick Piggin
2005-08-18  2:57                   ` Andi Kleen
2005-08-18  3:00                     ` Nick Piggin
2005-08-18  1:39               ` Coywolf Qi Hunt
2005-08-18  6:51                 ` Eric Dumazet
2005-08-18  9:05               ` [PATCH] Put the very large file_ra_state outside of 'struct file' Eric Dumazet

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=20050817.194315.111196480.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=ak@suse.de \
    --cc=bcrl@linux.intel.com \
    --cc=dada1@cosmosbay.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@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).