* Re: [PATCH] [504/2many] MAINTAINERS - USB PEGASUS DRIVER
From: Petko Manolov @ 2007-08-13 12:55 UTC (permalink / raw)
To: joe; +Cc: linux-usb-devel, petkan, netdev, linux-kernel, akpm, torvalds
In-Reply-To: <46bffc64.Are6NTp/45DX6lxZ%joe@perches.com>
On Sun, 12 Aug 2007, joe@perches.com wrote:
> Add file pattern to MAINTAINER entry
>
> Signed-off-by: Joe Perches <joe@perches.com>
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d822865..fc87fa7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4764,6 +4764,7 @@ L: linux-usb-devel@lists.sourceforge.net
> L: netdev@vger.kernel.org
> W: http://pegasus2.sourceforge.net/
> S: Maintained
> +F: drivers/net/usb/pegasus.*
>
> USB PRINTER DRIVER (usblp)
> P: Pete Zaitcev
>
Well, what can i say? "NO, don't do it!" is probably not going to work so
i assume i will have to ACK. :-)
Petko
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
^ permalink raw reply
* [kj] is_power_of_2 in net/pcmcia/pcnet_cs.c
From: vignesh babu @ 2007-08-13 12:52 UTC (permalink / raw)
To: dahinds, p_gortmaker; +Cc: netdev, Kernel Janitors List
Replacing n & (n - 1) for power of 2 check by is_power_of_2(n)
Signed-off-by: vignesh babu <vignesh.babu@wipro.com>
---
diff --git a/drivers/net/pcmcia/pcnet_cs.c b/drivers/net/pcmcia/pcnet_cs.c
index 63de89e..d23bf6e 100644
--- a/drivers/net/pcmcia/pcnet_cs.c
+++ b/drivers/net/pcmcia/pcnet_cs.c
@@ -36,6 +36,7 @@
#include <linux/string.h>
#include <linux/timer.h>
#include <linux/delay.h>
+#include <linux/log2.h>
#include <linux/ethtool.h>
#include <linux/netdevice.h>
#include <../drivers/net/8390.h>
@@ -1486,7 +1487,7 @@ static int setup_shmem_window(struct pcmcia_device *link, int start_pg,
window_size = 32 * 1024;
/* Make sure it's a power of two. */
- while ((window_size & (window_size - 1)) != 0)
+ while (!is_power_of_2(window_size))
window_size += window_size & ~(window_size - 1);
/* Allocate a memory window */
--
Vignesh Babu BM
_____________________________________________________________
"Why is it that every time I'm with you, makes me believe in magic?"
^ permalink raw reply related
* Re: [PATCH 2/4] Net: mac80211, remove bitfields from struct ieee80211_txrx_data
From: Johannes Berg @ 2007-08-13 12:44 UTC (permalink / raw)
To: Jiri Slaby
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, jbenc-AlSwsSmVLrQ,
flamingice-R9e9/4HEdknk1uMJSBkQmQ, jeff-o2qLIJkoznsdnm+yROfE0A
In-Reply-To: <2653412025907217317-2EuRcrBQ8V0@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 585 bytes --]
On Sun, 2007-08-12 at 15:08 +0200, Jiri Slaby wrote:
> - unsigned int fragmented:1; /* whether the MSDU was fragmented */
> + /* whether the MSDU was fragmented */
> +#define IEEE80211_TXRXD_FRAGMENTED BIT(0)
> +#define IEEE80211_TXRXD_TXUNICAST BIT(1)
> +#define IEEE80211_TXRXD_TXPS_BUFFERED BIT(2)
> +#define IEEE80211_TXRXD_TXPROBE_LAST_FRAG BIT(3)
> +#define IEEE80211_TXRXD_RXIN_SCAN BIT(4)
I know this isn't the style currently used, but could you put these
definitions before the struct declaration? That way, kerneldoc doesn't
totally screw up.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]
^ permalink raw reply
* Re: [RFD] Layering: Use-Case Composers (was: DRBD - what is it, anyways? [compare with e.g. NBD + MD raid])
From: David Greaves @ 2007-08-13 12:43 UTC (permalink / raw)
To: david
Cc: Paul Clements, Jan Engelhardt, Al Boldi, linux-kernel,
linux-fsdevel, netdev, linux-raid
In-Reply-To: <Pine.LNX.4.64.0708130127260.22470@asgard.lang.hm>
david@lang.hm wrote:
>> Would this just be relevant to network devices or would it improve
>> support for jostled usb and sata hot-plugging I wonder?
>
> good question, I suspect that some of the error handling would be
> similar (for devices that are unreachable not haning the system for
> example), but a lot of the rest would be different (do you really want
> to try to auto-resync to a drive that you _think_ just reappeared,
Well, omit 'think' and the answer may be "yes". A lot of systems are quite
simple and RAID is common on the desktop now. If jostled USB fits into this
category - then "yes".
> what
> if it's a different drive? how can you be sure?
And that's the key isn't it. We have the RAID device UUID and the superblock
info. Isn't that enough? If not then given the work involved an extended
superblock wouldn't be unreasonable.
And I suspect the capability of devices would need recording in the superblock
too? eg 'retry-on-fail'
I can see how md would fail a device but may now periodically retry it. If a
retry shows that it's back then it would validate it (UUID) and then resync it.
> ) the error rate of a
> network is gong to be significantly higher then for USB or SATA drives
> (although I suppose iscsi would be limilar)
I do agree - I was looking for value-add for the existing subsystem. If this
benefits existing RAID users then it's more likely to be attractive.
David
^ permalink raw reply
* Re: [PATCH 1/4] Net: mac80211, remove bitfields from struct ieee80211_tx_packet_data
From: Johannes Berg @ 2007-08-13 12:43 UTC (permalink / raw)
To: Jiri Slaby
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, jbenc-AlSwsSmVLrQ,
flamingice-R9e9/4HEdknk1uMJSBkQmQ, jeff-o2qLIJkoznsdnm+yROfE0A
In-Reply-To: <32694265142306417777-2EuRcrBQ8V0@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 551 bytes --]
On Sun, 2007-08-12 at 15:08 +0200, Jiri Slaby wrote:
> + if (control->flags & IEEE80211_TXCTL_REQ_TX_STATUS)
> + pkt_data->flags |= IEEE80211_TXPD_REQ_TX_STATUS;
> + if (control->flags & IEEE80211_TXCTL_DO_NOT_ENCRYPT)
> + pkt_data->flags |= IEEE80211_TXPD_DO_NOT_ENCRYPT;
> + if (control->flags & IEEE80211_TXCTL_REQUEUE)
> + pkt_data->flags |= IEEE80211_TXPD_REQUEUE;
> + if (control->type == IEEE80211_IF_TYPE_MGMT)
> + pkt_data->flags |= IEEE80211_TXPD_MGMT_IFACE;
This looks weird. Can't we just use the same flags?
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]
^ permalink raw reply
* Re: Block device throttling [Re: Distributed storage.]
From: Evgeniy Polyakov @ 2007-08-13 12:24 UTC (permalink / raw)
To: Daniel Phillips
Cc: Jens Axboe, netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <200708130518.14974.phillips@phunq.net>
On Mon, Aug 13, 2007 at 05:18:14AM -0700, Daniel Phillips (phillips@phunq.net) wrote:
> > If limit is for
> > 1gb of pending block io, and system has for example 2gbs of ram (or
> > any other resonable parameters), then there is no way we can deadlock
> > in allocation, since it will not force page reclaim mechanism.
>
> The problem is that sk_alloc (called from our block driver via
> socket->write) would recurse into shrink_pages, which recursively
> submits IO to our block driver and blocks on the throttle. Subtle
> indeed, and yet another demonstration of why vm recursion is a Bad
> Thing.
>
> I will find a traceback for you tomorrow, which makes this deadlock much
> clearer.
I see how it can happen, but device throttling is a solution we are
trying to complete, which main aim _is_ to remove this problem.
Lower per-device limit, so that the rest of the RAM allowed to
allocate all needed data structures in the network path.
Above example just has 1gb of ram, which should be enough for skbs, if
it is not, decrease limit to 500 mb and so on, until weighted load of
the system allows to always have a forward progress.
--
Evgeniy Polyakov
^ permalink raw reply
* Re: Block device throttling [Re: Distributed storage.]
From: Daniel Phillips @ 2007-08-13 12:18 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: Jens Axboe, netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <20070813120417.GA5992@2ka.mipt.ru>
On Monday 13 August 2007 05:04, Evgeniy Polyakov wrote:
> On Mon, Aug 13, 2007 at 04:04:26AM -0700, Daniel Phillips
(phillips@phunq.net) wrote:
> > On Monday 13 August 2007 01:14, Evgeniy Polyakov wrote:
> > > > Oops, and there is also:
> > > >
> > > > 3) The bio throttle, which is supposed to prevent deadlock, can
> > > > itself deadlock. Let me see if I can remember how it goes.
> > > >
> > > > * generic_make_request puts a bio in flight
> > > > * the bio gets past the throttle and initiates network IO
> > > > * net calls sk_alloc->alloc_pages->shrink_caches
> > > > * shrink_caches submits a bio recursively to our block device
> > > > * this bio blocks on the throttle
> > > > * net may never get the memory it needs, and we are wedged
> > >
> > > If system is in such condition, it is already broken - throttle
> > > limit must be lowered (next time) not to allow such situation.
> >
> > Agreed that the system is broken, however lowering the throttle
> > limit gives no improvement in this case.
>
> How is it ever possible? The whole idea of throttling is to remove
> such situation, and now you say it can not be solved.
It was solved, by not throttling writeout that comes from shrink_caches.
Ugly.
> If limit is for
> 1gb of pending block io, and system has for example 2gbs of ram (or
> any other resonable parameters), then there is no way we can deadlock
> in allocation, since it will not force page reclaim mechanism.
The problem is that sk_alloc (called from our block driver via
socket->write) would recurse into shrink_pages, which recursively
submits IO to our block driver and blocks on the throttle. Subtle
indeed, and yet another demonstration of why vm recursion is a Bad
Thing.
I will find a traceback for you tomorrow, which makes this deadlock much
clearer.
Regards
^ permalink raw reply
* Re: Block device throttling [Re: Distributed storage.]
From: Evgeniy Polyakov @ 2007-08-13 12:18 UTC (permalink / raw)
To: Daniel Phillips
Cc: Jens Axboe, netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <200708130418.03667.phillips@phunq.net>
On Mon, Aug 13, 2007 at 04:18:03AM -0700, Daniel Phillips (phillips@phunq.net) wrote:
> > No. Since all requests for virtual device end up in physical devices,
> > which have limits, this mechanism works. Virtual device will
> > essentially call either generic_make_request() for new physical
> > device (and thus will sleep is limit is over), or will process bios
> > directly, but in that case it will sleep in generic_make_request()
> > for virutal device.
>
> What can happen is, as soon as you unthrottle the previous queue,
> another thread can come in and put another request on it. Sure, that
> thread will likely block on the physical throttle and so will the rest
> of the incoming threads, but it still allows the higher level queue to
> grow past any given limit, with the help of lots of threads. JVM for
> example?
No. You get one slot, and one thread will not be blocked, all others
will. If lucky thread wants to put two requests it will be blocked on
second request, since underlying physical device does not accept requests
anymore an thus caller will sleep.
> Say you have a device mapper device with some physical device sitting
> underneath, the classic use case for this throttle code. Say 8,000
> threads each submit an IO in parallel. The device mapper mapping
> function will be called 8,000 times with associated resource
> allocations, regardless of any throttling on the physical device queue.
Each thread will sleep in generic_make_request(), if limit is specified
correctly, then allocated number of bios will be enough to have a
progress.
Here is an example:
let's say system has 20.000 pages in RAM and 20.000 in swap,
we have 8.000 threads, each one allocates a page, then next page and so
on. System has one virtual device with two physical devices under it,
each device gets half of requests.
We set limit to 4.000 per physical device.
All threads allocate a page and queue it to devices, so all threads
succeeded in its first allocation, and each device has its queue full.
Virtual device does not have a limit (or have it 4.000 too, but since it
was each time recharged, it has zero blocks in-flight).
New thread tries to allocate a page, it is allocated and queued to one
of the devices, but since its queue is full, thread sleeps. So will do
each other.
Thus we ended up allocated 8.000 requests queued, and 8.000 in-flight,
totally 16.000 which is smaller than amount of pages in RAM, so we are
happy.
Consider above as a special kind calculation i.e. number of
_allocated_ pages is always number of physical device multiplied by each
one's in-flight limit. By adjusting in-flight limit and knowing number
of device it is completely possible to eliminate vm deadlock.
If you do not like such calculation, solution is trivial:
we can sleep _after_ ->make_request_fn() in
generic_make_request() until number of in-flight bios is reduced by
bio_endio().
--
Evgeniy Polyakov
^ permalink raw reply
* Re: [PATCH] [374/2many] MAINTAINERS - PCNET32 NETWORK DRIVER
From: Jiri Kosina @ 2007-08-13 12:08 UTC (permalink / raw)
To: David Miller
Cc: joe, Linus Torvalds, pcnet32, netdev, linux-kernel, Andrew Morton
In-Reply-To: <20070812.233630.00455011.davem@davemloft.net>
On Sun, 12 Aug 2007, David Miller wrote:
> Ok, 374 patches is just rediculious.
> So many patches eats up an enormous amount of mailing list resources,
> and for these patches in particular there are few reasons to split them
> up at all. The fact that the split up landed you at 300+ patches is a
> very good indication of that.
Actually, I can see 546 patches up to now. Horrid.
Besides that, it's very unfortunate that every single mail of this
ridiculously huuuuge patch series starts a new thread (no 'References' or
'In-Reply-To' header), so it's not possible to nuke it at once in an usual
way.
--
Jiri Kosina
^ permalink raw reply
* Re: Block device throttling [Re: Distributed storage.]
From: Evgeniy Polyakov @ 2007-08-13 12:04 UTC (permalink / raw)
To: Daniel Phillips
Cc: Jens Axboe, netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <200708130404.26572.phillips@phunq.net>
On Mon, Aug 13, 2007 at 04:04:26AM -0700, Daniel Phillips (phillips@phunq.net) wrote:
> On Monday 13 August 2007 01:14, Evgeniy Polyakov wrote:
> > > Oops, and there is also:
> > >
> > > 3) The bio throttle, which is supposed to prevent deadlock, can
> > > itself deadlock. Let me see if I can remember how it goes.
> > >
> > > * generic_make_request puts a bio in flight
> > > * the bio gets past the throttle and initiates network IO
> > > * net calls sk_alloc->alloc_pages->shrink_caches
> > > * shrink_caches submits a bio recursively to our block device
> > > * this bio blocks on the throttle
> > > * net may never get the memory it needs, and we are wedged
> >
> > If system is in such condition, it is already broken - throttle limit
> > must be lowered (next time) not to allow such situation.
>
> Agreed that the system is broken, however lowering the throttle limit
> gives no improvement in this case.
How is it ever possible? The whole idea of throttling is to remove such
situation, and now you say it can not be solved. If limit is for 1gb of
pending block io, and system has for example 2gbs of ram (or any other
resonable parameters), then there is no way we can deadlock in
allocation, since it will not force page reclaim mechanism.
--
Evgeniy Polyakov
^ permalink raw reply
* Re: Distributed storage.
From: Daniel Phillips @ 2007-08-13 11:45 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: Jens Axboe, netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <20070813110302.GA28360@2ka.mipt.ru>
On Monday 13 August 2007 04:03, Evgeniy Polyakov wrote:
> On Mon, Aug 13, 2007 at 03:12:33AM -0700, Daniel Phillips
(phillips@phunq.net) wrote:
> > > This is not a very good solution, since it requires all users of
> > > the bios to know how to free it.
> >
> > No, only the specific ->endio needs to know that, which is set by
> > the bio owner, so this knowledge lies in exactly the right place.
> > A small handful of generic endios all with the same destructor are
> > used nearly everywhere.
>
> That is what I meant - there will be no way to just alloc a bio and
> put it, helpers for generic bio sets must be exported and each and
> every bi_end_io() must be changed to check reference counter and they
> must know how they were allocated.
There are fewer non-generic bio allocators than you think.
> Endio callback is of course quite rare and additional atomic
> reading will not kill the system, but why introduce another read?
> It is possible to provide a flag for endio callback that it is last,
> but it still requires to change every single callback - why do we
> want this?
We don't. Struct bio does not need to be shrunk. Jens wanted to talk
about what fields could be eliminated if we wanted to shrink it. It is
about time to let that lie, don't you think?
> So, I'm a bit lost...
>
> You say it is too big
Did not say that.
> and some parts can be removed or combined
True.
> and then that size does not matter.
Also true, backed up by numbers on real systems.
> Last/not-last checks in the code is
> not clear design, so I do not see why it is needed at all if not for
> size shrinking.
Not needed, indeed. Accurate throttling is needed. If the best way to
throttle requires expanding struct bio a little then we should not let
concerns about the cost of an int or two stand in the way. Like Jens,
I am more concerned about the complexity cost, and that is minimized in
my opinion by throttling in the generic code rather than with custom
code in each specialized block driver.
Your patch does throttle in the generic code, great. Next thing is to
be sure that it completely closes the window for reserve leakage, which
is not yet clear.
Regards,
Daniel
^ permalink raw reply
* Re: Block device throttling [Re: Distributed storage.]
From: Daniel Phillips @ 2007-08-13 11:18 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: Jens Axboe, netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <20070813082326.GC30089@2ka.mipt.ru>
On Monday 13 August 2007 01:23, Evgeniy Polyakov wrote:
> On Sun, Aug 12, 2007 at 10:36:23PM -0700, Daniel Phillips
(phillips@phunq.net) wrote:
> > (previous incomplete message sent accidentally)
> >
> > On Wednesday 08 August 2007 02:54, Evgeniy Polyakov wrote:
> > > On Tue, Aug 07, 2007 at 10:55:38PM +0200, Jens Axboe wrote:
> > >
> > > So, what did we decide? To bloat bio a bit (add a queue pointer)
> > > or to use physical device limits? The latter requires to replace
> > > all occurence of bio->bi_bdev = something_new with
> > > blk_set_bdev(bio, somthing_new), where queue limits will be
> > > appropriately charged. So far I'm testing second case, but I only
> > > changed DST for testing, can change all other users if needed
> > > though.
> >
> > Adding a queue pointer to struct bio and using physical device
> > limits as in your posted patch both suffer from the same problem:
> > you release the throttling on the previous queue when the bio moves
> > to a new one, which is a bug because memory consumption on the
> > previous queue then becomes unbounded, or limited only by the
> > number of struct requests that can be allocated. In other words,
> > it reverts to the same situation we have now as soon as the IO
> > stack has more than one queue. (Just a shorter version of my
> > previous post.)
>
> No. Since all requests for virtual device end up in physical devices,
> which have limits, this mechanism works. Virtual device will
> essentially call either generic_make_request() for new physical
> device (and thus will sleep is limit is over), or will process bios
> directly, but in that case it will sleep in generic_make_request()
> for virutal device.
What can happen is, as soon as you unthrottle the previous queue,
another thread can come in and put another request on it. Sure, that
thread will likely block on the physical throttle and so will the rest
of the incoming threads, but it still allows the higher level queue to
grow past any given limit, with the help of lots of threads. JVM for
example?
Say you have a device mapper device with some physical device sitting
underneath, the classic use case for this throttle code. Say 8,000
threads each submit an IO in parallel. The device mapper mapping
function will be called 8,000 times with associated resource
allocations, regardless of any throttling on the physical device queue.
Anyway, your approach is awfully close to being airtight, there is just
a small hole. I would be more than happy to be proved wrong about
that, but the more I look, the more I see that hole.
> > 1) One throttle count per submitted bio is too crude a measure. A
> > bio can carry as few as one page or as many as 256 pages. If you
> > take only
>
> It does not matter - we can count bytes, pages, bio vectors or
> whatever we like, its just a matter of counter and can be changed
> without problem.
Quite true. In some cases the simple inc/dec per bio works just fine.
But the general case where finer granularity is required comes up in
existing code, so there needs to be a plan.
Regards,
Daniel
^ permalink raw reply
* Re: Block device throttling [Re: Distributed storage.]
From: Daniel Phillips @ 2007-08-13 11:04 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: Jens Axboe, netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <20070813081441.GA30089@2ka.mipt.ru>
On Monday 13 August 2007 01:14, Evgeniy Polyakov wrote:
> > Oops, and there is also:
> >
> > 3) The bio throttle, which is supposed to prevent deadlock, can
> > itself deadlock. Let me see if I can remember how it goes.
> >
> > * generic_make_request puts a bio in flight
> > * the bio gets past the throttle and initiates network IO
> > * net calls sk_alloc->alloc_pages->shrink_caches
> > * shrink_caches submits a bio recursively to our block device
> > * this bio blocks on the throttle
> > * net may never get the memory it needs, and we are wedged
>
> If system is in such condition, it is already broken - throttle limit
> must be lowered (next time) not to allow such situation.
Agreed that the system is broken, however lowering the throttle limit
gives no improvement in this case.
This is not theoretical, but a testable, repeatable result.
Instructions to reproduce should show up tomorrow.
This bug is now solved in a kludgy way. Now, Peter's patch set offers a
much cleaner way to fix this little problem, along with at least one
other nasty that it already fixed.
Regards,
Daniel
^ permalink raw reply
* Re: Distributed storage.
From: Evgeniy Polyakov @ 2007-08-13 11:03 UTC (permalink / raw)
To: Daniel Phillips
Cc: Jens Axboe, netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <200708130312.33903.phillips@phunq.net>
On Mon, Aug 13, 2007 at 03:12:33AM -0700, Daniel Phillips (phillips@phunq.net) wrote:
> > This is not a very good solution, since it requires all users of the
> > bios to know how to free it.
>
> No, only the specific ->endio needs to know that, which is set by the
> bio owner, so this knowledge lies in exactly the right place. A small
> handful of generic endios all with the same destructor are used nearly
> everywhere.
That is what I meant - there will be no way to just alloc a bio and put
it, helpers for generic bio sets must be exported and each and every
bi_end_io() must be changed to check reference counter and they must
know how they were allocated.
> > Right now it is hidden.
> > And adds additional atomic check (although reading is quite fast) in
> > the end_io.
>
> Actual endio happens once in the lifetime of the transfer, this read
> will be entirely lost in the noise.
Not always. Sometimes it is called multiple times, but all bi_end_io()
callbacks I checked (I believe all in mainline tree) tests if bi_size is
zero or not.
Endio callback is of course quite rare and additional atomic
reading will not kill the system, but why introduce another read?
It is possible to provide a flag for endio callback that it is last, but
it still requires to change every single callback - why do we want this?
> > And for what purpose? To eat 8 bytes on 64bit platform?
> > This will not reduce its size noticebly, so the same number of bios
> > will be in the cache's page, so what is a gain? All this cleanups and
> > logic complicatins should be performed only if after size shring
> > increased number of bios can fit into cache's page, will it be done
> > after such cleanups?
>
> Well, exactly, My point from the beginning was that the size of struct
> bio is not even close to being a problem and adding a few bytes to it
> in the interest of doing the cleanest fix to a core kernel bug is just
> not a dominant issue.
So, I'm a bit lost...
You say it is too big and some parts can be removed or combined, and
then that size does not matter. Last/not-last checks in the code is not
clear design, so I do not see why it is needed at all if not for size
shrinking.
> I suppose that leaving out the word "bloated" and skipping straight to
> the "doesn't matter" proof would have saved some bandwidth.
:) Likely it will.
--
Evgeniy Polyakov
^ permalink raw reply
* Re: [PATCH] [70/2many] MAINTAINERS - ARPD SUPPORT
From: Alan Cox @ 2007-08-13 10:49 UTC (permalink / raw)
To: joe; +Cc: torvalds, netdev, linux-kernel, akpm
In-Reply-To: <46bff8e8.HhTQ7TBPy1zkcNfh%joe@perches.com>
On Sun, 12 Aug 2007 23:23:36 -0700
joe@perches.com wrote:
> Add file pattern to MAINTAINER entry
>
> Signed-off-by: Joe Perches <joe@perches.com>
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 90c1b81..ac2226b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -697,6 +697,7 @@ ARPD SUPPORT
> P: Jonathan Layes
> L: netdev@vger.kernel.org
> S: Maintained
> +F: net/ipv4/arp.c
NAK
arp.c is the netdev people, ARPD is a corner case bit of code within it,
something your patterns don't actually cope with
^ permalink raw reply
* Re: Distributed storage.
From: Daniel Phillips @ 2007-08-13 10:32 UTC (permalink / raw)
To: Jens Axboe
Cc: Evgeniy Polyakov, netdev, linux-kernel, linux-fsdevel,
Peter Zijlstra
In-Reply-To: <20070813102255.GN23758@kernel.dk>
On Monday 13 August 2007 03:22, Jens Axboe wrote:
> I never compared the bio to struct page, I'd obviously agree that
> shrinking struct page was a worthy goal and that it'd be ok to uglify
> some code to do that. The same isn't true for struct bio.
I thought I just said that.
Regards,
Daniel
^ permalink raw reply
* Re: Distributed storage.
From: Jens Axboe @ 2007-08-13 10:22 UTC (permalink / raw)
To: Daniel Phillips
Cc: Evgeniy Polyakov, netdev, linux-kernel, linux-fsdevel,
Peter Zijlstra
In-Reply-To: <200708130315.08382.phillips@phunq.net>
On Mon, Aug 13 2007, Daniel Phillips wrote:
> On Monday 13 August 2007 03:06, Jens Axboe wrote:
> > On Mon, Aug 13 2007, Daniel Phillips wrote:
> > > Of course not. Nothing I said stops endio from being called in the
> > > usual way as well. For this to work, endio just needs to know that
> > > one call means "end" and the other means "destroy", this is
> > > trivial.
> >
> > Sorry Daniel, but your suggestions would do nothing more than uglify
> > the code and design.
>
> Pretty much exactly what was said about shrinking struct page, ask Bill.
> The difference was, shrinking struct page actually mattered whereas
> shrinking struct bio does not, and neither does expanding it by a few
> bytes.
Lets back this up a bit - this whole thing began with you saying that
struct bio was bloated already, which I said wasn't true. You then
continued to hand wave your wave through various suggestions to trim the
obvious fat from that structure, none of which were nice or feasible.
I never compared the bio to struct page, I'd obviously agree that
shrinking struct page was a worthy goal and that it'd be ok to uglify
some code to do that. The same isn't true for struct bio.
And we can expand struct bio if we have to, naturally. And I've done it
before, which I wrote in the initial mail. I just don't want to do it
casually, then it WILL be bloated all of a sudden. Your laissez faire
attitude towards adding members to struct bio "oh I'll just add it and
someone less lazy than me will fix it up in the future" makes me happy
that you are not maintaining anything that I use.
I'll stop replying to your mails until something interesting surfaces.
I've already made my points clear about both the above and the
throttling. And I'd advise you to let Evgeniy take this forward, he
seems a lot more adept to actually getting CODE done and - at least from
my current and past perspective - is someone you can actually have a
fruitful conversation with.
--
Jens Axboe
^ permalink raw reply
* Re: Distributed storage.
From: Daniel Phillips @ 2007-08-13 10:15 UTC (permalink / raw)
To: Jens Axboe
Cc: Evgeniy Polyakov, netdev, linux-kernel, linux-fsdevel,
Peter Zijlstra
In-Reply-To: <20070813100636.GL23758@kernel.dk>
On Monday 13 August 2007 03:06, Jens Axboe wrote:
> On Mon, Aug 13 2007, Daniel Phillips wrote:
> > Of course not. Nothing I said stops endio from being called in the
> > usual way as well. For this to work, endio just needs to know that
> > one call means "end" and the other means "destroy", this is
> > trivial.
>
> Sorry Daniel, but your suggestions would do nothing more than uglify
> the code and design.
Pretty much exactly what was said about shrinking struct page, ask Bill.
The difference was, shrinking struct page actually mattered whereas
shrinking struct bio does not, and neither does expanding it by a few
bytes.
Regards,
Daniel
^ permalink raw reply
* drivers/net/tokenring/3c359.c
From: Surya Prabhakar N @ 2007-08-13 10:13 UTC (permalink / raw)
To: mikep, netdev; +Cc: linux-tr, Linux Kernel, kernel-janitors
Hi,
Replacing kmalloc with kzalloc and cleaning up memset in
drivers/net/tokenring/3c359.c
Signed-off-by: Surya Prabhakar <surya.prabhakar@wipro.com>
---
diff --git a/drivers/net/tokenring/3c359.c b/drivers/net/tokenring/3c359.c
index 9f1b6ab..d36dd53 100644
--- a/drivers/net/tokenring/3c359.c
+++ b/drivers/net/tokenring/3c359.c
@@ -641,14 +641,14 @@ static int xl_open(struct net_device *dev)
* Now to set up the Rx and Tx buffer structures
*/
/* These MUST be on 8 byte boundaries */
- xl_priv->xl_tx_ring = kmalloc((sizeof(struct xl_tx_desc) * XL_TX_RING_SIZE) + 7, GFP_DMA | GFP_KERNEL) ;
+ xl_priv->xl_tx_ring = kzalloc((sizeof(struct xl_tx_desc) * XL_TX_RING_SIZE) + 7, GFP_DMA | GFP_KERNEL) ;
if (xl_priv->xl_tx_ring == NULL) {
printk(KERN_WARNING "%s: Not enough memory to allocate rx buffers.\n",
dev->name);
free_irq(dev->irq,dev);
return -ENOMEM;
}
- xl_priv->xl_rx_ring = kmalloc((sizeof(struct xl_rx_desc) * XL_RX_RING_SIZE) +7, GFP_DMA | GFP_KERNEL) ;
+ xl_priv->xl_rx_ring = kzalloc((sizeof(struct xl_rx_desc) * XL_RX_RING_SIZE) +7, GFP_DMA | GFP_KERNEL) ;
if (xl_priv->xl_tx_ring == NULL) {
printk(KERN_WARNING "%s: Not enough memory to allocate rx buffers.\n",
dev->name);
@@ -656,8 +656,6 @@ static int xl_open(struct net_device *dev)
kfree(xl_priv->xl_tx_ring);
return -ENOMEM;
}
- memset(xl_priv->xl_tx_ring,0,sizeof(struct xl_tx_desc) * XL_TX_RING_SIZE) ;
- memset(xl_priv->xl_rx_ring,0,sizeof(struct xl_rx_desc) * XL_RX_RING_SIZE) ;
/* Setup Rx Ring */
for (i=0 ; i < XL_RX_RING_SIZE ; i++) {
--
thanks
surya.
^ permalink raw reply related
* Re: Distributed storage.
From: Daniel Phillips @ 2007-08-13 10:12 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: Jens Axboe, netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <20070813091856.GA5503@2ka.mipt.ru>
On Monday 13 August 2007 02:18, Evgeniy Polyakov wrote:
> On Mon, Aug 13, 2007 at 02:08:57AM -0700, Daniel Phillips
(phillips@phunq.net) wrote:
> > > But that idea fails as well, since reference counts and IO
> > > completion are two completely seperate entities. So unless end IO
> > > just happens to be the last user holding a reference to the bio,
> > > you cannot free it.
> >
> > That is not a problem. When bio_put hits zero it calls ->endio
> > instead of the destructor. The ->endio sees that the count is zero
> > and destroys the bio.
>
> This is not a very good solution, since it requires all users of the
> bios to know how to free it.
No, only the specific ->endio needs to know that, which is set by the
bio owner, so this knowledge lies in exactly the right place. A small
handful of generic endios all with the same destructor are used nearly
everywhere.
> Right now it is hidden.
> And adds additional atomic check (although reading is quite fast) in
> the end_io.
Actual endio happens once in the lifetime of the transfer, this read
will be entirely lost in the noise.
> And for what purpose? To eat 8 bytes on 64bit platform?
> This will not reduce its size noticebly, so the same number of bios
> will be in the cache's page, so what is a gain? All this cleanups and
> logic complicatins should be performed only if after size shring
> increased number of bios can fit into cache's page, will it be done
> after such cleanups?
Well, exactly, My point from the beginning was that the size of struct
bio is not even close to being a problem and adding a few bytes to it
in the interest of doing the cleanest fix to a core kernel bug is just
not a dominant issue.
I suppose that leaving out the word "bloated" and skipping straight to
the "doesn't matter" proof would have saved some bandwidth.
Regards,
Daniel
^ permalink raw reply
* Re: Distributed storage.
From: Jens Axboe @ 2007-08-13 10:06 UTC (permalink / raw)
To: Daniel Phillips
Cc: Evgeniy Polyakov, netdev, linux-kernel, linux-fsdevel,
Peter Zijlstra
In-Reply-To: <200708130255.35828.phillips@phunq.net>
On Mon, Aug 13 2007, Daniel Phillips wrote:
> On Monday 13 August 2007 02:13, Jens Axboe wrote:
> > On Mon, Aug 13 2007, Daniel Phillips wrote:
> > > On Monday 13 August 2007 00:45, Jens Axboe wrote:
> > > > On Mon, Aug 13 2007, Jens Axboe wrote:
> > > > > > You did not comment on the one about putting the bio
> > > > > > destructor in the ->endio handler, which looks dead simple.
> > > > > > The majority of cases just use the default endio handler and
> > > > > > the default destructor. Of the remaining cases, where a
> > > > > > specialized destructor is needed, typically a specialized
> > > > > > endio handler is too, so combining is free. There are few if
> > > > > > any cases where a new specialized endio handler would need to
> > > > > > be written.
> > > > >
> > > > > We could do that without too much work, I agree.
> > > >
> > > > But that idea fails as well, since reference counts and IO
> > > > completion are two completely seperate entities. So unless end IO
> > > > just happens to be the last user holding a reference to the bio,
> > > > you cannot free it.
> > >
> > > That is not a problem. When bio_put hits zero it calls ->endio
> > > instead of the destructor. The ->endio sees that the count is zero
> > > and destroys the bio.
> >
> > You can't be serious? You'd stall end io completion notification
> > because someone holds a reference to a bio.
>
> Of course not. Nothing I said stops endio from being called in the
> usual way as well. For this to work, endio just needs to know that one
> call means "end" and the other means "destroy", this is trivial.
Sorry Daniel, but your suggestions would do nothing more than uglify the
code and design.
--
Jens Axboe
^ permalink raw reply
* [KJ] replacing kmalloc with kzalloc in drivers/net/sb1250-mac.c
From: Surya Prabhakar N @ 2007-08-13 9:58 UTC (permalink / raw)
To: netdev; +Cc: Linux Kernel, Linux Kernel
Hi,
Replacing kmalloc with kzalloc and cleaning up memset in
drivers/net/sb1250-mac.c
Signed-off-by: Surya Prabhakar <surya.prabhakar@wipro.com>
---
diff --git a/drivers/net/sb1250-mac.c b/drivers/net/sb1250-mac.c
index e7fdcf1..2dca5a7 100644
--- a/drivers/net/sb1250-mac.c
+++ b/drivers/net/sb1250-mac.c
@@ -760,7 +760,7 @@ static void sbdma_initctx(sbmacdma_t *d,
d->sbdma_dscrtable_unaligned =
d->sbdma_dscrtable = (sbdmadscr_t *)
- kmalloc((d->sbdma_maxdescr+1)*sizeof(sbdmadscr_t), GFP_KERNEL);
+ kzalloc((d->sbdma_maxdescr+1)*sizeof(sbdmadscr_t), GFP_KERNEL);
/*
* The descriptor table must be aligned to at least 16 bytes or the
@@ -769,8 +769,6 @@ static void sbdma_initctx(sbmacdma_t *d,
d->sbdma_dscrtable = (sbdmadscr_t *)
ALIGN((unsigned long)d->sbdma_dscrtable, sizeof(sbdmadscr_t));
- memset(d->sbdma_dscrtable,0,d->sbdma_maxdescr*sizeof(sbdmadscr_t));
-
d->sbdma_dscrtable_end = d->sbdma_dscrtable + d->sbdma_maxdescr;
d->sbdma_dscrtable_phys = virt_to_phys(d->sbdma_dscrtable);
@@ -780,9 +778,7 @@ static void sbdma_initctx(sbmacdma_t *d,
*/
d->sbdma_ctxtable = (struct sk_buff **)
- kmalloc(d->sbdma_maxdescr*sizeof(struct sk_buff *), GFP_KERNEL);
-
- memset(d->sbdma_ctxtable,0,d->sbdma_maxdescr*sizeof(struct sk_buff *));
+ kzalloc(d->sbdma_maxdescr*sizeof(struct sk_buff *), GFP_KERNEL);
#ifdef CONFIG_SBMAC_COALESCE
/*
--
thanks
surya.
^ permalink raw reply related
* Re: Distributed storage.
From: Daniel Phillips @ 2007-08-13 9:55 UTC (permalink / raw)
To: Jens Axboe
Cc: Evgeniy Polyakov, netdev, linux-kernel, linux-fsdevel,
Peter Zijlstra
In-Reply-To: <20070813091348.GJ23758@kernel.dk>
On Monday 13 August 2007 02:13, Jens Axboe wrote:
> On Mon, Aug 13 2007, Daniel Phillips wrote:
> > On Monday 13 August 2007 00:45, Jens Axboe wrote:
> > > On Mon, Aug 13 2007, Jens Axboe wrote:
> > > > > You did not comment on the one about putting the bio
> > > > > destructor in the ->endio handler, which looks dead simple.
> > > > > The majority of cases just use the default endio handler and
> > > > > the default destructor. Of the remaining cases, where a
> > > > > specialized destructor is needed, typically a specialized
> > > > > endio handler is too, so combining is free. There are few if
> > > > > any cases where a new specialized endio handler would need to
> > > > > be written.
> > > >
> > > > We could do that without too much work, I agree.
> > >
> > > But that idea fails as well, since reference counts and IO
> > > completion are two completely seperate entities. So unless end IO
> > > just happens to be the last user holding a reference to the bio,
> > > you cannot free it.
> >
> > That is not a problem. When bio_put hits zero it calls ->endio
> > instead of the destructor. The ->endio sees that the count is zero
> > and destroys the bio.
>
> You can't be serious? You'd stall end io completion notification
> because someone holds a reference to a bio.
Of course not. Nothing I said stops endio from being called in the
usual way as well. For this to work, endio just needs to know that one
call means "end" and the other means "destroy", this is trivial.
Regards,
Daniel
^ permalink raw reply
* Re: Distributed storage.
From: Evgeniy Polyakov @ 2007-08-13 9:18 UTC (permalink / raw)
To: Daniel Phillips
Cc: Jens Axboe, netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <200708130208.57542.phillips@phunq.net>
On Mon, Aug 13, 2007 at 02:08:57AM -0700, Daniel Phillips (phillips@phunq.net) wrote:
> > But that idea fails as well, since reference counts and IO completion
> > are two completely seperate entities. So unless end IO just happens
> > to be the last user holding a reference to the bio, you cannot free
> > it.
>
> That is not a problem. When bio_put hits zero it calls ->endio instead
> of the destructor. The ->endio sees that the count is zero and
> destroys the bio.
This is not a very good solution, since it requires all users of the
bios to know how to free it. Right now it is hidden.
And adds additional atomic check (although reading is quite fast) in the
end_io. And for what purpose? To eat 8 bytes on 64bit platform? This
will not reduce its size noticebly, so the same number of bios will be
in the cache's page, so what is a gain? All this cleanups and logic
complicatins should be performed only if after size shring increased
number of bios can fit into cache's page, will it be done after such
cleanups?
--
Evgeniy Polyakov
^ permalink raw reply
* Re: [PATCH 9/24] make atomic_read() behave consistently on ia64
From: Geert Uytterhoeven @ 2007-08-13 7:39 UTC (permalink / raw)
To: Chris Snook
Cc: Paul Mackerras, Linus Torvalds, Luck, Tony, Andreas Schwab,
linux-kernel, linux-arch, netdev, akpm, ak, heiko.carstens, davem,
schwidefsky, wensong, horms, wjiang, cfriesen, zlynx, rpjday,
jesper.juhl
In-Reply-To: <46BFFA71.4080601@redhat.com>
On Mon, 13 Aug 2007, Chris Snook wrote:
> Paul Mackerras wrote:
> > Chris Snook writes:
> > > I'll do this for the whole patchset. Stay tuned for the resubmit.
> >
> > Could you incorporate Segher's patch to turn atomic_{read,set} into
> > asm on powerpc? Segher claims that using asm is really the only
> > reliable way to ensure that gcc does what we want, and he seems to
> > have a point.
> >
> > Paul.
>
> I haven't seen a patch yet. I'm going to resubmit with inline volatile-cast
> atomic[64]_[read|set] on all architectures as a reference point, and if anyone
> wants to go and implement some of them in assembly, that's between them and
> the relevant arch maintainers. I have no problem with (someone else) doing it
> in assembly. I just don't think it's necessary and won't let it hold up the
> effort to get consistent behavior on all architectures.
http://lkml.org/lkml/2007/8/10/470
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox