Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 17 Feb 2003 18:57:54 -0700
From:      "Justin T. Gibbs" <gibbs@scsiguy.com>
To:        Maxime Henrion <mux@freebsd.org>
Cc:        Sam Leffler <sam@freebsd.org>, Perforce Change Reviews <perforce@freebsd.org>
Subject:   Re: PERFORCE change 24844 for review
Message-ID:  <658530000.1045533474@aslan.scsiguy.com>
In-Reply-To: <20030218011913.GF60813@elvis.mu.org>
References:  <200302082030.h18KUuR9063864@repoman.freebsd.org> <47870000.1045456722@aslan.scsiguy.com> <20030218011913.GF60813@elvis.mu.org>

next in thread | previous in thread | raw e-mail | index | archive | help
> Justin T. Gibbs wrote:
>> > http://perforce.freebsd.org/chv.cgi?CH=24844
>> > 
>> > Change 24844 by sam@sam_ebb on 2003/02/08 12:30:08
>> > 
>> > 	bus_dma'd em driver (works on x86, not on sparc)
>> > 	replace rx logic with jumbo buffers (needs more work)
>> There are lots of things that are a bit wierd about how this
>> was bus dma'ed...
> [...]
>> o bus dmamaps have a lifetime of a single packet.  While loading a map
>>   is usually cheap, allocating a map may be very costly.  The implementation
>>   may use the allocation of a map as a hint to increase its mapping space,
>>   bounce buffer pool, or to perform some other action that will decrease
>>   the chance that a load will block should all maps currently allocated
>>   against the tag be used at once.  The map may be a fairly large object
>>   depending on the needs of the implementation.  I would suggest changing
>>   the maps to have the same lifetime as the TX descriptors.
> [...]
> 
> Are you just talking about TX descriptors, or also RX ones?

It should also apply to the RX descriptors.

> Having dma
> maps with the same lifetime as the descriptor is easy for the TX case,
> but it's actually quite hard for the RX case.  FWIW,  When I converted
> the xl(4) driver to busdma, I made it so RX dma maps have the same
> lifetime as the RX descriptor, and was only doing bus_dmamap_load() when
> mapping another packet, but I will probably soon change this and create
> a new dma map for each packet.  The reason for this is the way we handle
> memory shortage in the drivers.  When xl_newbuf() fails, we don't pass
> the packet to the upper layer and instead reuse the mbuf to get another
> packet.  That's OK in the non-busdma case, but in the busdma case, we
> need to handle another possible failure with the bus_dmamap_load() call.
> The problem is that we already did call bus_dmamap_unload() and the
> "old" mbuf isn't mapped anymore.  And since we can't do selective
> bus_dmamap_unload() calls, we can't just unmap it after we succeded in
> mapping the new mbuf.
> 
> It would probably be better to fix the busdma API so that we can
> selectively unload one mapping from a map, but until we got this feature
> I'll move the map creation at mbuf allocation time.

If you are asking that the API allow something other than a 1-1 map to
mapping relationship, this would just mean that the implementation would
have to allocate a "map like object" to attach to the map in order to
do the extra map operations.  The whole point of the 1-1 relationship
is to ensure that these resources are pre-allocated before the mapping
operation is required.  Selectively unmapping portions of a map would
also needlessly complicate the API and/or implementations of the API.

> Fixing this problem
> differently without touching the busdma code implies having an RX
> descriptors list for those descriptors we failed to map, so that we can
> try to use them again later.

I think you are exagerating the difficulty of solving this problem.
You need one more map than the total number of RX descriptors.  Store
this extra map in the softc.  In xl_newbuf(), map the mbuf using the softc
dmamap.  If the mapping is successful, swap the softc map with the map
in the target tx descriptor.  The caller can then unload using the
softc map.

> This has far-reaching consequences, as
> we'll have to modify the RX descriptor chain, and many drivers
> statically chain the descriptors for performance reasons...  IMHO that's
> way too much complexity.
> 
> Of course, I would be happy to be told that I missed some point and
> there's an even better way of fixing this than what I'm currently
> planning on committing :-).

Let me know if the above doesn't work for you.

--
Justin


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe p4-projects" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?658530000.1045533474>