bufq massage
this diff is largely a mechanical change.
firstly, it makes struct bufq a member of the softc for devices
that use it, rather than it being a pointer to something that needs
to be allocated at attach. since all these devices need a bufq to
operate, it makes sense to have it allocated as part of the softc
and get bufq_init to just initialise all its fields. it also gets
rid of the possibility that you wont be able to allocate teh bufq
struct during attach, which is something you dont want to happen.
secondly, it consistently implements a split between wrapper functions
and the per discipline implementation of the bufq handlers. it
consistently does the locking in the wrappers rather than doing
half in the wrappers and the other half in the implementations.
it also consistently handles the outstanding bufq bq pointer in the
wrappers.
this hides most of the implementation inside kern_bufq.c. the only
stuff left in buf.h is for the bits each implementation needs to
put bufs on their queues.
ive tested this extensively on sd(4) and thib has tested this on
wd(4). we'd like some wider exposure, especially over suspends and
resumes on a variety of machines. i have tried to preserve the
locking semantics, but testing would be lovely.
ok?
dlg
Index: dev/ata/wd.c
===
RCS file: /cvs/src/sys/dev/ata/wd.c,v
retrieving revision 1.85
diff -u -p -r1.85 wd.c
--- dev/ata/wd.c28 Jun 2010 08:35:46 - 1.85
+++ dev/ata/wd.c25 Aug 2010 12:05:33 -
@@ -121,7 +121,7 @@ struct wd_softc {
/* General disk infos */
struct device sc_dev;
struct disk sc_dk;
- struct bufq *sc_bufq;
+ struct bufq sc_bufq;
/* IDE disk soft states */
struct ata_bio sc_wdc_bio; /* current transfer */
@@ -369,7 +369,7 @@ wdattach(struct device *parent, struct d
*/
wd-sc_dk.dk_driver = wddkdriver;
wd-sc_dk.dk_name = wd-sc_dev.dv_xname;
- wd-sc_bufq = bufq_init(BUFQ_DEFAULT);
+ bufq_init(wd-sc_bufq, BUFQ_DEFAULT);
wd-sc_sdhook = shutdownhook_establish(wd_shutdown, wd);
if (wd-sc_sdhook == NULL)
printf(%s: WARNING: unable to establish shutdown hook\n,
@@ -413,7 +413,7 @@ wddetach(struct device *self, int flags)
/* Remove unprocessed buffers from queue */
s = splbio();
- while ((bp = BUFQ_DEQUEUE(sc-sc_bufq)) != NULL) {
+ while ((bp = bufq_dequeue(sc-sc_bufq)) != NULL) {
bp-b_error = ENXIO;
bp-b_flags |= B_ERROR;
biodone(bp);
@@ -435,7 +435,7 @@ wddetach(struct device *self, int flags)
shutdownhook_disestablish(sc-sc_sdhook);
/* Detach disk. */
- bufq_destroy(sc-sc_bufq);
+ bufq_destroy(sc-sc_bufq);
disk_detach(sc-sc_dk);
return (0);
@@ -486,7 +486,7 @@ wdstrategy(struct buf *bp)
(wd-sc_flags (WDF_WLABEL|WDF_LABELLING)) != 0) = 0)
goto done;
/* Queue transfer on drive, activate drive and controller if idle. */
- BUFQ_QUEUE(wd-sc_bufq, bp);
+ bufq_queue(wd-sc_bufq, bp);
s = splbio();
wdstart(wd);
splx(s);
@@ -518,7 +518,7 @@ wdstart(void *arg)
while (wd-openings 0) {
/* Is there a buf for us ? */
- if ((bp = BUFQ_DEQUEUE(wd-sc_bufq)) == NULL)
+ if ((bp = bufq_dequeue(wd-sc_bufq)) == NULL)
return;
/*
* Make the command. First lock the device
Index: kern/kern_bufq.c
===
RCS file: /cvs/src/sys/kern/kern_bufq.c,v
retrieving revision 1.14
diff -u -p -r1.14 kern_bufq.c
--- kern/kern_bufq.c19 Jul 2010 21:39:15 - 1.14
+++ kern/kern_bufq.c25 Aug 2010 12:05:33 -
@@ -30,45 +30,70 @@ SLIST_HEAD(, bufq) bufqs = SLIST_HEAD_IN
struct mutex bufqs_mtx = MUTEX_INITIALIZER(IPL_NONE);
intbufqs_stop;
-struct buf *(*bufq_dequeuev[BUFQ_HOWMANY])(struct bufq *, int) = {
- bufq_disksort_dequeue,
- bufq_fifo_dequeue
+struct bufq_impl {
+ void*(*impl_create)(void);
+ void (*impl_destroy)(void *);
+
+ void (*impl_queue)(void *, struct buf *);
+ struct buf *(*impl_dequeue)(void *);
+ void (*impl_requeue)(void *, struct buf *);
+ int (*impl_peek)(void *);
};
-void (*bufq_queuev[BUFQ_HOWMANY])(struct bufq *, struct buf *) = {
+
+void *bufq_disksort_create(void);
+voidbufq_disksort_destroy(void *);
+voidbufq_disksort_queue(void *, struct buf *);
+struct buf *bufq_disksort_dequeue(void *);
+voidbufq_disksort_requeue(void *, struct buf *);
+int bufq_disksort_peek(void *);
+
+struct bufq_impl bufq_impl_disksort = {
+ bufq_disksort_create,
+
Re: bufq massage
On Sun, Aug 29, 2010 at 10:26:14PM +1000, David Gwynne wrote:
this diff is largely a mechanical change.
firstly, it makes struct bufq a member of the softc for devices
that use it, rather than it being a pointer to something that needs
to be allocated at attach. since all these devices need a bufq to
operate, it makes sense to have it allocated as part of the softc
and get bufq_init to just initialise all its fields. it also gets
rid of the possibility that you wont be able to allocate teh bufq
struct during attach, which is something you dont want to happen.
secondly, it consistently implements a split between wrapper functions
and the per discipline implementation of the bufq handlers. it
consistently does the locking in the wrappers rather than doing
half in the wrappers and the other half in the implementations.
it also consistently handles the outstanding bufq bq pointer in the
wrappers.
this hides most of the implementation inside kern_bufq.c. the only
stuff left in buf.h is for the bits each implementation needs to
put bufs on their queues.
ive tested this extensively on sd(4) and thib has tested this on
wd(4). we'd like some wider exposure, especially over suspends and
resumes on a variety of machines. i have tried to preserve the
locking semantics, but testing would be lovely.
ok?
I like this a lot, and I will do some testing on st and cd at least as
soon as I can rebuild the appropriate box.
Ken
dlg
Index: dev/ata/wd.c
===
RCS file: /cvs/src/sys/dev/ata/wd.c,v
retrieving revision 1.85
diff -u -p -r1.85 wd.c
--- dev/ata/wd.c 28 Jun 2010 08:35:46 - 1.85
+++ dev/ata/wd.c 25 Aug 2010 12:05:33 -
@@ -121,7 +121,7 @@ struct wd_softc {
/* General disk infos */
struct device sc_dev;
struct disk sc_dk;
- struct bufq *sc_bufq;
+ struct bufq sc_bufq;
/* IDE disk soft states */
struct ata_bio sc_wdc_bio; /* current transfer */
@@ -369,7 +369,7 @@ wdattach(struct device *parent, struct d
*/
wd-sc_dk.dk_driver = wddkdriver;
wd-sc_dk.dk_name = wd-sc_dev.dv_xname;
- wd-sc_bufq = bufq_init(BUFQ_DEFAULT);
+ bufq_init(wd-sc_bufq, BUFQ_DEFAULT);
wd-sc_sdhook = shutdownhook_establish(wd_shutdown, wd);
if (wd-sc_sdhook == NULL)
printf(%s: WARNING: unable to establish shutdown hook\n,
@@ -413,7 +413,7 @@ wddetach(struct device *self, int flags)
/* Remove unprocessed buffers from queue */
s = splbio();
- while ((bp = BUFQ_DEQUEUE(sc-sc_bufq)) != NULL) {
+ while ((bp = bufq_dequeue(sc-sc_bufq)) != NULL) {
bp-b_error = ENXIO;
bp-b_flags |= B_ERROR;
biodone(bp);
@@ -435,7 +435,7 @@ wddetach(struct device *self, int flags)
shutdownhook_disestablish(sc-sc_sdhook);
/* Detach disk. */
- bufq_destroy(sc-sc_bufq);
+ bufq_destroy(sc-sc_bufq);
disk_detach(sc-sc_dk);
return (0);
@@ -486,7 +486,7 @@ wdstrategy(struct buf *bp)
(wd-sc_flags (WDF_WLABEL|WDF_LABELLING)) != 0) = 0)
goto done;
/* Queue transfer on drive, activate drive and controller if idle. */
- BUFQ_QUEUE(wd-sc_bufq, bp);
+ bufq_queue(wd-sc_bufq, bp);
s = splbio();
wdstart(wd);
splx(s);
@@ -518,7 +518,7 @@ wdstart(void *arg)
while (wd-openings 0) {
/* Is there a buf for us ? */
- if ((bp = BUFQ_DEQUEUE(wd-sc_bufq)) == NULL)
+ if ((bp = bufq_dequeue(wd-sc_bufq)) == NULL)
return;
/*
* Make the command. First lock the device
Index: kern/kern_bufq.c
===
RCS file: /cvs/src/sys/kern/kern_bufq.c,v
retrieving revision 1.14
diff -u -p -r1.14 kern_bufq.c
--- kern/kern_bufq.c 19 Jul 2010 21:39:15 - 1.14
+++ kern/kern_bufq.c 25 Aug 2010 12:05:33 -
@@ -30,45 +30,70 @@ SLIST_HEAD(, bufq)bufqs = SLIST_HEAD_IN
struct mutex bufqs_mtx = MUTEX_INITIALIZER(IPL_NONE);
int bufqs_stop;
-struct buf *(*bufq_dequeuev[BUFQ_HOWMANY])(struct bufq *, int) = {
- bufq_disksort_dequeue,
- bufq_fifo_dequeue
+struct bufq_impl {
+ void*(*impl_create)(void);
+ void (*impl_destroy)(void *);
+
+ void (*impl_queue)(void *, struct buf *);
+ struct buf *(*impl_dequeue)(void *);
+ void (*impl_requeue)(void *, struct buf *);
+ int (*impl_peek)(void *);
};
-void (*bufq_queuev[BUFQ_HOWMANY])(struct bufq *, struct buf *) = {
+
+void *bufq_disksort_create(void);
+void bufq_disksort_destroy(void *);
+void bufq_disksort_queue(void *, struct buf *);
+struct buf *bufq_disksort_dequeue(void *);
+void
He get $219,249 Last Month
He get $219,249 Last Month Works on ANY Computer Profiting 112 Minutes From NOW $4,191 In First 7 Days $219,249 Last Month Only 234 Copies Available - If You Don't Get Results, You'll Get A Full Refund AND $100 Join Now : http://thenewsdetail.info
Calling all pcmcia card owners
Please test this diff on all pcmcia cards and machines you can find,
and report the results directly to me (ie. test the zaurus with brand
new -current code, apm i386 laptops, and the other non-suspending
cases such as amd64/i386 acpi - but don't test suspend there).
PLEASE do this very soon since I want to move ahead quickly (I am
working on making all 4 (zaurus, i386-apm, i386-acpi, amd64-acpi)
suspend-resume codepaths use the same driver infastructure; this will
remove a butload of special cases.
This will not yet make pcmcia suspend/resume correctly on many acpi
machines; that requires more diffs which will come once pcmcia is
fixed. That's a hint to test this on the other machines soon. There
are also some usb changes coming, and then zaurus changes, and then
i386 changes fully using the new infrastructure.. and finally, removal
of all the powerhook code.
I know there are many bugs left in here; there were many here to start
with. This diff will be split up when it is commited, but it is better
to have it all for testing.
A rough summary of the changes:
- pcmcia was converting suspend/resume into activate/deactivate to the
card drivers. this is WRONG, since the needs of activate and deactivate
are different. I have started trying to resolve this in some drivers,
and want to get this diff in with a few fixed drivers, so that the others
can catch up to the new better way of doing this
- some drivers do the pcmcia_function_enable() and interrupt establishment
in their xxattach, then do it again in xxactivate. I am trying to work
around this now, but not yet happy with the solution
- related to the previous comment, a massive amount of #ifdef notyet
code in these drivers will be deleted later on, but I wanted to keep
this simple for now.
- i found a few more drivers which need a workq for resume
- to interlock against the ioctl, kettenis wants us to move the
tsleep(sc-wi_flags thing to a rw_lock; someone has to do that work
in some other drivers first then I will copy that here.
- there are other drivers in other subsystems which use workq and need
the interlocks too. is anyone going to help?
- pcmcia com continue talking serial in cu after an unsuspend
- i have been playing with looping ifconfig's and ping's through suspends
- ne works
- wi works
- malo is causing memory corruption on a re-insert and re-dhclient. I
cannot spot the bug.
- wdc controllers work fine; no more wdc timeout errors!
- wdc at install time requires that softraid be disabled because
softraid is walking alldevs searching for disks and then calling a
bucketload of functions which sleep. It is a massive bug in softraid.
- sm, xe, an, ray are untested, but I will attempt to write diffs for
them if someone shows up with hardware
Index: pci/if_wi_pci.c
===
RCS file: /cvs/src/sys/dev/pci/if_wi_pci.c,v
retrieving revision 1.46
diff -u -p -u -r1.46 if_wi_pci.c
--- pci/if_wi_pci.c 27 Aug 2010 18:29:44 - 1.46
+++ pci/if_wi_pci.c 29 Aug 2010 18:06:17 -
@@ -187,7 +187,18 @@ wi_pci_resume(void *arg1, void *arg2)
{
struct wi_softc *sc = (struct wi_softc *)arg1;
+ int s;
+
+ s = splnet();
+ while (sc-wi_flags WI_FLAGS_BUSY)
+ tsleep(sc-wi_flags, 0, wipwr, 0);
+ sc-wi_flags |= WI_FLAGS_BUSY;
+
wi_init(sc);
+
+ sc-wi_flags = ~WI_FLAGS_BUSY;
+ wakeup(sc-wi_flags);
+ splx(s);
}
void
Index: ic/if_wi.c
===
RCS file: /cvs/src/sys/dev/ic/if_wi.c,v
retrieving revision 1.148
diff -u -p -u -r1.148 if_wi.c
--- ic/if_wi.c 2 Jul 2010 02:40:15 - 1.148
+++ ic/if_wi.c 29 Aug 2010 18:24:33 -
@@ -1544,28 +1544,36 @@ STATIC int
wi_ioctl(struct ifnet *ifp, u_long command, caddr_t data)
{
int s, error = 0, i, j, len;
- struct wi_softc *sc;
- struct ifreq*ifr;
+ struct wi_softc *sc = ifp-if_softc;
+ struct ifreq*ifr = (struct ifreq *)data;
struct proc *p = curproc;
struct ifaddr *ifa = (struct ifaddr *)data;
struct wi_scan_res *res;
struct wi_scan_p2_hdr *p2;
struct wi_req *wreq = NULL;
u_int32_t flags;
-
struct ieee80211_nwid *nwidp = NULL;
struct ieee80211_nodereq_all*na;
struct ieee80211_bssid *bssid;
s = splnet();
-
- sc = ifp-if_softc;
- ifr = (struct ifreq *)data;
-
if (!(sc-wi_flags WI_FLAGS_ATTACHED)) {
+ error = ENODEV;
+ goto fail;
+ }
+
+ /*
+* Prevent processes from entering this function while another
+* process is tsleep'ing in it.
+*/
+ while ((sc-wi_flags WI_FLAGS_BUSY) error == 0)
+ error = tsleep(sc-wi_flags,
Re: Backout mclgeti for vr(4).
unless someone fixes mclgeti in this driver in the next 24 hours, this should
go in.
this has my ok on august 31.
On 28/08/2010, at 4:07 AM, Thordur I Bjornsson wrote:
As seen on misc@, and also by myself sometime ago, but I forgot about
it as work got hectic and I was moving.
Anyways, vr(4) will not even surivie a few ping -f's, before the pools
become corrupt, this has (obviously) something todo with how MCLGETI
takes and puts buf's of the rings;
Reverting MCLGETI fixes the issue. I've attached a diff, I remember
testing it, and it survives fine.
I did spent some time back then trying to figure this out but to no avail.
The problem is the card is still messing with mbufs apperently after they
have been taken of the ring (This is kind of, similar to rev1.94 I
think).
Index: dev/pci/if_vr.c
===
RCS file: /home/cvs/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.105
diff -u -p -r1.105 if_vr.c
--- dev/pci/if_vr.c 19 May 2010 15:27:35 - 1.105
+++ dev/pci/if_vr.c 5 Aug 2010 15:59:05 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: if_vr.c,v 1.105 2010/05/19 15:27:35 oga Exp $ */
+/* $OpenBSD: if_vr.c,v 1.95 2009/06/04 16:56:20 sthen Exp $*/
/*
* Copyright (c) 1997, 1998
@@ -135,10 +135,9 @@ void vr_setcfg(struct vr_softc *, int);
void vr_iff(struct vr_softc *);
void vr_reset(struct vr_softc *);
int vr_list_rx_init(struct vr_softc *);
-void vr_fill_rx_ring(struct vr_softc *);
int vr_list_tx_init(struct vr_softc *);
-int vr_alloc_mbuf(struct vr_softc *, struct vr_chain_onefrag *);
+int vr_alloc_mbuf(struct vr_softc *, struct vr_chain_onefrag *, struct mbuf
*);
/*
* Supported devices quirks
@@ -664,7 +663,6 @@ vr_attach(struct device *parent, struct
/*
* Call MI attach routines.
*/
- m_clsetwms(ifp, MCLBYTES, 2, VR_RX_LIST_CNT - 1);
if_attach(ifp);
ether_ifattach(ifp);
return;
@@ -749,6 +747,9 @@ vr_list_rx_init(struct vr_softc *sc)
sc-sc_listmap-dm_segs[0].ds_addr +
offsetof(struct vr_list_data, vr_rx_list[i]);
+ if (vr_alloc_mbuf(sc, cd-vr_rx_chain[i], NULL))
+ return (ENOBUFS);
+
if (i == (VR_RX_LIST_CNT - 1))
nexti = 0;
else
@@ -760,30 +761,11 @@ vr_list_rx_init(struct vr_softc *sc)
offsetof(struct vr_list_data, vr_rx_list[nexti]));
}
- cd-vr_rx_prod = cd-vr_rx_cons = cd-vr_rx_chain[0];
- cd-vr_rx_cnt = 0;
- vr_fill_rx_ring(sc);
+ cd-vr_rx_head = cd-vr_rx_chain[0];
return (0);
}
-void
-vr_fill_rx_ring(struct vr_softc *sc)
-{
- struct vr_chain_data*cd;
- struct vr_list_data *ld;
-
- cd = sc-vr_cdata;
- ld = sc-vr_ldata;
-
- while (cd-vr_rx_cnt VR_RX_LIST_CNT) {
- if (vr_alloc_mbuf(sc, cd-vr_rx_prod))
- break;
- cd-vr_rx_prod = cd-vr_rx_prod-vr_nextdesc;
- cd-vr_rx_cnt++;
- }
-}
-
/*
* A frame has been uploaded: pass the resulting mbuf chain up to
* the higher level protocols.
@@ -791,7 +773,7 @@ vr_fill_rx_ring(struct vr_softc *sc)
void
vr_rxeof(struct vr_softc *sc)
{
- struct mbuf *m;
+ struct mbuf *m0, *m;
struct ifnet*ifp;
struct vr_chain_onefrag *cur_rx;
int total_len = 0;
@@ -799,21 +781,20 @@ vr_rxeof(struct vr_softc *sc)
ifp = sc-arpcom.ac_if;
- while(sc-vr_cdata.vr_rx_cnt 0) {
+ for (;;) {
+
bus_dmamap_sync(sc-sc_dmat, sc-sc_listmap,
0, sc-sc_listmap-dm_mapsize,
BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
- rxstat = letoh32(sc-vr_cdata.vr_rx_cons-vr_ptr-vr_status);
+ rxstat = letoh32(sc-vr_cdata.vr_rx_head-vr_ptr-vr_status);
if (rxstat VR_RXSTAT_OWN)
break;
- rxctl = letoh32(sc-vr_cdata.vr_rx_cons-vr_ptr-vr_ctl);
+ rxctl = letoh32(sc-vr_cdata.vr_rx_head-vr_ptr-vr_ctl);
- cur_rx = sc-vr_cdata.vr_rx_cons;
- m = cur_rx-vr_mbuf;
- cur_rx-vr_mbuf = NULL;
- sc-vr_cdata.vr_rx_cons = cur_rx-vr_nextdesc;
- sc-vr_cdata.vr_rx_cnt--;
+ m0 = NULL;
+ cur_rx = sc-vr_cdata.vr_rx_head;
+ sc-vr_cdata.vr_rx_head = cur_rx-vr_nextdesc;
/*
* If an error occurs, update stats, clear the
@@ -843,13 +824,24 @@ vr_rxeof(struct vr_softc *sc)
printf(\n);
#endif
- m_freem(m);
+ /* Reinitialize descriptor */
+ cur_rx-vr_ptr-vr_status = htole32(VR_RXSTAT);
+ cur_rx-vr_ptr-vr_data =
+ htole32(cur_rx-vr_map-dm_segs[0].ds_addr +
+
Re: Backout mclgeti for vr(4).
unless someone fixes mclgeti in this driver in the next 24 hours, this should go in. this has my ok on august 31. It is a very nasty bug. But we should also remember that machines with vr(4) interfaces are pretty slow, and benefit tremendously from MCLGETI. So it is a crying shame to see it go.
