(no subject)
Hello.
Help me please to understand how the function bus_teardown_intr() works.
I have a device driver containing following code:
#define DEVICE2SOFTC(device) ((struct dev_softc *)device_get_softc(device))
static void dev_intr(void *arg);
struct dev_softc {
...
int rid_irq;
struct resource* res_irq;
void *intr_cookie;
...
};
static int
dev_attach(device_t device)
{
...
dev_sc-rid_irq = 0;
dev_sc-res_irq = bus_alloc_resource_any(device, SYS_RES_IRQ,
(dev_sc-rid_irq), RF_SHAREABLE|RF_ACTIVE);
if (dev_sc-res_irq == NULL)
{
uprintf(!!! Could not map interrupt !!!\n);
goto fail;
}
if (bus_setup_intr(device, dev_sc-res_irq, INTR_TYPE_TTY,
dev_intr, dev_sc, dev_sc-intr_cookie))
{
uprintf(!!! Could not setup irq !!!\n);
goto fail;
}
...
fail:
return ENXIO;
}
static int
dev_detach(device_t device)
{
struct dev_softc *dev_sc = DEVICE2SOFTC(device);
destroy_dev(dev_sc-device);
if (bus_teardown_intr(device, dev_sc-res_irq, dev_sc-intr_cookie) != 0);
printf(bus_teardown_intr ERROR !!!\n);
bus_release_resource(device, SYS_RES_IRQ, dev_sc-rid_irq,
dev_sc-res_irq);
...
return 0;
}
static void
dev_intr(void *arg)
{
struct dev_softc *dev_sc = (struct dev_softc *)arg;
...
}
When the driver is loaded the following message is shown:
dev0 port 0x9800-0x980f,0x9400-0x947f irq 16 at device 10.0 on pci1
i.e., as I understand, resourses are allocated normally.
But when the driver is being unloaded the following message appear:
bus_teardown_intr ERROR !!!
What have I done wrong? Hint me please how to use bus_teardown_intr()
function correctly?
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
bus_teardown_intr()
Sorry, I forgot to indicate the theme.
Hello.
Help me please to understand how the function bus_teardown_intr() works.
I have a device driver containing following code:
#define DEVICE2SOFTC(device) ((struct dev_softc *)device_get_softc(device))
static void dev_intr(void *arg);
struct dev_softc {
...
int rid_irq;
struct resource* res_irq;
void *intr_cookie;
...
};
static int
dev_attach(device_t device)
{
...
dev_sc-rid_irq = 0;
dev_sc-res_irq = bus_alloc_resource_any(device, SYS_RES_IRQ,
(dev_sc-rid_irq), RF_SHAREABLE|RF_ACTIVE);
if (dev_sc-res_irq == NULL)
{
uprintf(!!! Could not map interrupt !!!\n);
goto fail;
}
if (bus_setup_intr(device, dev_sc-res_irq, INTR_TYPE_TTY,
dev_intr, dev_sc, dev_sc-intr_cookie))
{
uprintf(!!! Could not setup irq !!!\n);
goto fail;
}
...
fail:
return ENXIO;
}
static int
dev_detach(device_t device)
{
struct dev_softc *dev_sc = DEVICE2SOFTC(device);
destroy_dev(dev_sc-device);
if (bus_teardown_intr(device, dev_sc-res_irq, dev_sc-intr_cookie) != 0);
printf(bus_teardown_intr ERROR !!!\n);
bus_release_resource(device, SYS_RES_IRQ, dev_sc-rid_irq,
dev_sc-res_irq);
...
return 0;
}
static void
dev_intr(void *arg)
{
struct dev_softc *dev_sc = (struct dev_softc *)arg;
...
}
When the driver is loaded the following message is shown:
dev0 port 0x9800-0x980f,0x9400-0x947f irq 16 at device 10.0 on pci1
i.e., as I understand, resourses are allocated normally.
But when the driver is being unloaded the following message appear:
bus_teardown_intr ERROR !!!
What have I done wrong? Hint me please how to use bus_teardown_intr()
function correctly?
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
--
Яндекс.Лента - любимые блоги и новости на одном сайте http://lenta.yandex.ru/
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
RE: (no subject)
#define DEVICE2SOFTC(device) ((struct dev_softc
*)device_get_softc(device))
static void dev_intr(void *arg);
struct dev_softc {
...
int rid_irq;
struct resource* res_irq;
void*intr_cookie;
...
};
static int
dev_attach(device_t device)
{
...
dev_sc-rid_irq = 0;
dev_sc-res_irq = bus_alloc_resource_any(device, SYS_RES_IRQ,
(dev_sc-rid_irq),
RF_SHAREABLE|RF_ACTIVE);
if (dev_sc-res_irq == NULL)
{
uprintf(!!! Could not map interrupt !!!\n);
goto fail;
}
if (bus_setup_intr(device, dev_sc-res_irq, INTR_TYPE_TTY,
dev_intr, dev_sc, dev_sc-intr_cookie))
{
uprintf(!!! Could not setup irq !!!\n);
goto fail;
}
...
fail:
return ENXIO;
}
static int
dev_detach(device_t device)
{
struct dev_softc *dev_sc = DEVICE2SOFTC(device);
destroy_dev(dev_sc-device);
if (bus_teardown_intr(device, dev_sc-res_irq,
dev_sc-intr_cookie) != 0);
Do you see that semicolon?
Norbert
printf(bus_teardown_intr ERROR !!!\n);
bus_release_resource(device, SYS_RES_IRQ, dev_sc-rid_irq,
dev_sc-res_irq);
...
return 0;
}
static void
dev_intr(void *arg)
{
struct dev_softc *dev_sc = (struct dev_softc *)arg;
...
}
When the driver is loaded the following message is shown:
dev0 port 0x9800-0x980f,0x9400-0x947f irq 16 at device 10.0 on pci1
i.e., as I understand, resourses are allocated normally.
But when the driver is being unloaded the following message appear:
bus_teardown_intr ERROR !!!
What have I done wrong? Hint me please how to use bus_teardown_intr()
function correctly?
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
RE: (no subject)
#define DEVICE2SOFTC(device) ((struct dev_softc
*)device_get_softc(device))
static void dev_intr(void *arg);
struct dev_softc {
...
int rid_irq;
struct resource* res_irq;
void *intr_cookie;
...
};
static int
dev_attach(device_t device)
{
...
dev_sc-rid_irq = 0;
dev_sc-res_irq = bus_alloc_resource_any(device, SYS_RES_IRQ,
(dev_sc-rid_irq),
RF_SHAREABLE|RF_ACTIVE);
if (dev_sc-res_irq == NULL)
{
uprintf(!!! Could not map interrupt !!!\n);
goto fail;
}
if (bus_setup_intr(device, dev_sc-res_irq, INTR_TYPE_TTY,
dev_intr, dev_sc, dev_sc-intr_cookie))
{
uprintf(!!! Could not setup irq !!!\n);
goto fail;
}
...
fail:
return ENXIO;
}
static int
dev_detach(device_t device)
{
struct dev_softc *dev_sc = DEVICE2SOFTC(device);
destroy_dev(dev_sc-device);
if (bus_teardown_intr(device, dev_sc-res_irq,
dev_sc-intr_cookie) != 0);
Do you see that semicolon?
Norbert
Oops... I am ashamed for my inattention...
Thank you very much...
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: How to do proper locking
On Wednesday 03 August 2005 19:21, John Baldwin wrote:
On Tuesday 02 August 2005 06:23 pm, Hans Petter Selasky wrote:
Hi,
I am looking for a safe way to access structures that can be freed. The
solution I am looking for must not:
- hinder scaleability
- lead to use of a single lock
- lead to lock order reversal
There is one more thing. When the structure is freed it should not block
waiting for any reference counts.
These aren't a very clear set of requirements.
If you think about the situation of accessing a structure in parallell with
freeing it, and want to do it 100 percent safe, you will see that you have to
do some kind of test before accessing this structure to see if it is still
present. So how should this test look like if you want to keep the points
I've listed above in mind?
Note that you can't hold a
lock across malloc(), so your allocation code isn't safe.
I was thinking that the M_NOWAIT flag was passed malloc(). Else in this case,
I think it is safe to unlock/lock lock_A when malloc is called.
You can try taking at look at some existing refcounted structures such
as ucreds, p_args, etc. or looking at structures held in a global list
like proc.
It looks to me like some parent lock is held, or that these structures are
only accessed from a single thread, to prevent synchronization problems.
This is a copy and paste from the kernel sources:
struct ucred *
crhold(struct ucred *cr)
{
The problem is, what happens if the kernel switches thread right here, and
then the other thread calls crfree() on this structure, that will free()
memory pointed to by cr. Then the first line of code below will access
freed memory, and then we are going for a segment fault or worse.
mtx_lock(cr-cr_mtxp);
cr-cr_ref++;
mtx_unlock(cr-cr_mtxp);
return (cr);
}
So this is what I want this routine and alike to look like:
struct cr_callback_args {
struct ucred *cr;
struct mtx *p_mtx;
u_int32_t refcount_copy,
u_int32_t *p_refcount;
};
struct ucred *
crhold(struct cr_ballback_args *args)
{
mtx_lock(args-p_mtx);
if(args-refcount_copy == ref_atomic_read(args-p_refcount))
{
/* the structure pointed to by args-cr is still valid */
/* maybe we don't need the refcount below, but instead
* can return with this structure locked, hence the
* reference count will be incremented before
* args-cr is freed, and after that the test
* above will fail.
*/
args-cr-cr_ref++;
}
mtx_lock(args-p_mtx);
return (args-cr);
}
What actual problem are you trying to solve?
Generic callbacks:
- setting up timers. Make sure that callback is never called after that timer
has been stopped.
- setting up devices. Make sure that read/write/ioctl/poll callbacks are never
called after that the device has been freed.
- setting up interrupts. Make sure that interrupt routine is never called
after that it is been teared down.
And more. There are lots of situations in the kernel that run into the same
scheme, without a proper solution.
In my initial suggestion, I suggested that there be a ref_atomic_increment()
routine, but I think it is better without. Then every time one needs to
increment the refcount, one has to go through a ref_free/ref_alloc loop. The
idea is that ref_free() will detect when the refcount is equal to
(u_int32_t)(-1) and put the refount on a trash list. Then ideally one never
recycles used up refcounts.
How does code obtain references to my_struct objects?
For example when you set up a timer, you pass the pointer to the structure
that the callback is going to access.
struct my_struct *sc = ...;
callout_reset(callback, 1*hz, my_callback, sc);
Are they hung off another object that has
a lock?
Usually, but the parent lock cannot help much.
Are they in a global list?
Yes and no. Sometimes these structures have roots in structures that are freed
too, in the same manner, but at the bottom there is a static and global list.
Was this what you meant?
-- HPS
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: How to do proper locking
On Thursday 04 August 2005 13:40, Hans Petter Selasky wrote:
This is a copy and paste from the kernel sources:
struct ucred *
crhold(struct ucred *cr)
{
The problem is, what happens if the kernel switches thread right here, and
then the other thread calls crfree() on this structure, that will
free() memory pointed to by cr. Then the first line of code below will
access freed memory, and then we are going for a segment fault or worse.
mtx_lock(cr-cr_mtxp);
cr-cr_ref++;
mtx_unlock(cr-cr_mtxp);
return (cr);
}
It seems that you are misunderstanding the concept of reference counting. The
idea is to have a reference as long as you are handing around the object. In
order to call crhold() you must already hold a reference to the credential.
This will prevent the credential structure to vanish and allow you to pass
the credential to another consumer that now can assume to hold a reference of
it's own.
If you can access your objects from a external list, you have to hold a global
lock that protects:
1) Sanity of the list
2) Free operations to the member objects
If you access the objects via the list/tree/whatsoever most of the time -
refcounting isn't for you.
--
/\ Best regards, | [EMAIL PROTECTED]
\ / Max Laier | ICQ #67774661
X http://pf4freebsd.love2party.net/ | [EMAIL PROTECTED]
/ \ ASCII Ribbon Campaign | Against HTML Mail and News
pgpH6ellm2xb0.pgp
Description: PGP signature
Sub-optimal libc's read-ahead buffering behaviour
Hi,
I have found the scenario in which our libc behaves utterly
suboptimally. Consider the following piece of code reads and processes
every other 512-bytes block in a file (error handling intentionally
omitted):
FILE *f;
int i;
char buf[512];
f = fopen(...);
for (i = 0; feof(f) == 0; i++) {
fread(buf, sizeof(buf), 1, f);
do_process(buf);
fseek(f, i * 2 * sizeof(buf), SEEK_SET);
}
What I have discovered in this case is that libc reads 4096 bytes from
the file for *each* fread(3) call, despite the fact that it can only do
one actual read(2) for every fourth fread(3) and satisfy the rest from
the internal buffer (4096 bytes). However, if I replace fseek(3) with
just another dummy fread(3) everything works as expected - libc does
only one read for every 8 fread(3) calls (4 dummy and 4 real).
Is it something which should be fixed or are there some subtle reasons
for the current behaviour?
Following is piece of code which illustrates the problem:
#include stdio.h
#include stdlib.h
int
main(int argc, char **argv)
{
FILE *f;
int i;
char buf[512];
f = fopen(/dev/zero, r);
for (i = 0; i 16; i++) {
fread(buf, sizeof(buf), 1, f);
if (argc == 1)
fread(buf, sizeof(buf), 1, f);
else
fseek(f, i * 2 * sizeof(buf), SEEK_SET);
}
exit(0);
}
When run with zero arguments relevant truss output looks like:
open(/dev/zero,0x0,0666) = 3 (0x3)
fstat(3,0xbfbfe900) = 0 (0x0)
readlink(/etc/malloc.conf,0xbfbfe8c0,63) ERR#2 'No such file or
directory'
issetugid() = 0 (0x0)
mmap(0x0,4096,(0x3)PROT_READ|PROT_WRITE,(0x1002)MAP_ANON|MAP_PRIVATE,-1,0x0)
= 1209335808 (0x4815)
break(0x804b000) = 0 (0x0)
break(0x804c000) = 0 (0x0)
ioctl(3,TIOCGETA,0xbfbfe940) ERR#19 'Operation not
supported by device'
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
exit(0x0)
While when I am specifying some argument it becomes:
open(/dev/zero,0x0,0666) = 3 (0x3)
fstat(3,0xbfbfe900) = 0 (0x0)
readlink(/etc/malloc.conf,0xbfbfe8c0,63) ERR#2 'No such file or
directory'
issetugid() = 0 (0x0)
mmap(0x0,4096,(0x3)PROT_READ|PROT_WRITE,(0x1002)MAP_ANON|MAP_PRIVATE,-1,0x0)
= 1209335808 (0x4815)
break(0x804b000) = 0 (0x0)
break(0x804c000) = 0 (0x0)
ioctl(3,TIOCGETA,0xbfbfe940) ERR#19 'Operation not
supported by device'
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x0,SEEK_SET)= 0 (0x0)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x400,SEEK_SET) = 1024 (0x400)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x800,SEEK_SET) = 2048 (0x800)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0xc00,SEEK_SET) = 3072 (0xc00)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x1000,SEEK_SET) = 4096 (0x1000)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x1400,SEEK_SET) = 5120 (0x1400)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x1800,SEEK_SET) = 6144 (0x1800)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x1c00,SEEK_SET) = 7168 (0x1c00)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x2000,SEEK_SET) = 8192 (0x2000)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x2400,SEEK_SET) = 9216 (0x2400)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x2800,SEEK_SET) = 10240 (0x2800)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x2c00,SEEK_SET) = 11264 (0x2c00)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x3000,SEEK_SET) = 12288 (0x3000)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x3400,SEEK_SET) = 13312 (0x3400)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x3800,SEEK_SET) = 14336 (0x3800)
read(0x3,0x804b000,0x1000) = 4096 (0x1000)
lseek(3,0x3c00,SEEK_SET) = 15360 (0x3c00)
exit(0x0)
The output speaks for
Re: How to do proper locking
On Thursday 04 August 2005 07:40 am, Hans Petter Selasky wrote:
On Wednesday 03 August 2005 19:21, John Baldwin wrote:
On Tuesday 02 August 2005 06:23 pm, Hans Petter Selasky wrote:
Hi,
I am looking for a safe way to access structures that can be freed. The
solution I am looking for must not:
- hinder scaleability
- lead to use of a single lock
- lead to lock order reversal
There is one more thing. When the structure is freed it should not block
waiting for any reference counts.
These aren't a very clear set of requirements.
If you think about the situation of accessing a structure in parallell with
freeing it, and want to do it 100 percent safe, you will see that you have
to do some kind of test before accessing this structure to see if it is
still present. So how should this test look like if you want to keep the
points I've listed above in mind?
Note that you can't hold a
lock across malloc(), so your allocation code isn't safe.
I was thinking that the M_NOWAIT flag was passed malloc(). Else in this
case, I think it is safe to unlock/lock lock_A when malloc is called.
You can try taking at look at some existing refcounted structures such
as ucreds, p_args, etc. or looking at structures held in a global list
like proc.
It looks to me like some parent lock is held, or that these structures are
only accessed from a single thread, to prevent synchronization problems.
You hold a parent lock (which is what your lock_A is if you think about it)
while you bump the reference count, yes.
This is a copy and paste from the kernel sources:
struct ucred *
crhold(struct ucred *cr)
{
The problem is, what happens if the kernel switches thread right here, and
then the other thread calls crfree() on this structure, that will
free() memory pointed to by cr. Then the first line of code below will
access freed memory, and then we are going for a segment fault or worse.
mtx_lock(cr-cr_mtxp);
cr-cr_ref++;
mtx_unlock(cr-cr_mtxp);
return (cr);
}
crhold() is always called when the caller knows that it has a reference to cr
that can't go away. Thus, in the case of p_ucred, one holds the associated
PROC_LOCK() while doing crhold(). After the crhold() you can drop the proc
lock, so that lock is only held for a very short period of time. You then
have your own reference to the cred structure that is valid until you call
crfree(). In the case of curthread-td_ucred, the reference is valid until
curthread releases it on a cred update at the start of a syscall, so your
reference is valid without needing locks for the life of a syscall. The key
here is that you solve this problem at a higher level, not within the
structure itself.
What actual problem are you trying to solve?
Generic callbacks:
- setting up timers. Make sure that callback is never called after that
timer has been stopped.
See callout_stop() and callout_drain(). callout_drain() won't return until
the callout has both been stopped and has finished executing if it was
already in progress.
- setting up devices. Make sure that read/write/ioctl/poll callbacks are
never called after that the device has been freed.
This is managed with refcounts on the dev_t object by devfs. Once you call
destroy_dev() devfs manages the rest.
- setting up interrupts. Make sure that interrupt routine is never called
after that it is been teared down.
bus_teardown_intr() already does this.
And more. There are lots of situations in the kernel that run into the same
scheme, without a proper solution.
Are there any other places you can think of that don't handle this already?
The approach FreeBSD's kernel has taken is to solve this problem by
duplicating a lot of checks all over the place for each data structure, but
to solve it in the primitives themselves instead (hence callout_drain(),
bus_teardown_intr(), destroy_dev(), etc.)
--
John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/
Power Users Use the Power to Serve = http://www.FreeBSD.org
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: How to do proper locking
On Thursday 04 August 2005 14:08, Max Laier wrote:
On Thursday 04 August 2005 13:40, Hans Petter Selasky wrote:
This is a copy and paste from the kernel sources:
struct ucred *
crhold(struct ucred *cr)
{
The problem is, what happens if the kernel switches thread right here,
and then the other thread calls crfree() on this structure, that will
free() memory pointed to by cr. Then the first line of code below
will access freed memory, and then we are going for a segment fault or
worse.
mtx_lock(cr-cr_mtxp);
cr-cr_ref++;
mtx_unlock(cr-cr_mtxp);
return (cr);
}
It seems that you are misunderstanding the concept of reference counting.
The idea is to have a reference as long as you are handing around the
object. In order to call crhold() you must already hold a reference to the
credential. This will prevent the credential structure to vanish and allow
you to pass the credential to another consumer that now can assume to hold
a reference of it's own.
Maybe crhold() is not exactly what I am after. From what I can see, we are
speaking about two different refcount systems:
- crhold()/crfree() is incrementing/decrementing the refcount to indicate the
total number of references hold.
- I am only incrementing the refcount at object destruction, to make sure that
the last refcount value cannot be used again, to get access to the object.
If you can access your objects from a external list, you have to hold a
global lock that protects:
1) Sanity of the list
2) Free operations to the member objects
Yes, that is what I am doing, but there is a problem. Here is another
pseudo-code example on what I'm trying to get a solution for:
struct mtx global;
struct my_object {
struct mtx *mtx;
u_int8_t some_data[256];
struct my_object *next;
} *m;
/* Here is the enforced locking order, which
* makes most sense to me:
*
* 1. m-mtx
* 2. global
*/
void()
thread_1()
{
struct my_object *m;
sleep(global);
lock(global);
/*
* to access this object
* we dequeue it from a
* one-way linked list:
*/
m = dequeue_object();
mtx = m-mtx;
unlock(global);
wakeup(m);
sleep(m);
/* What makes this so special is that
* the object pointed to by m is owned
* by thread_2. When that thread
* exits the object is gone.
*
* A big problem is that one cannot
* hold any lock right here, to
* block thread_2 from freeing the
* object, due to the enforced locking
* order. Switching locking order
* does not change anything.
*/
lock(mtx);
/* When the CPU gets here a check
* is neccesary to ensure that the
* other thread has not freed the
* object pointed to by m.
*/
if(refcount_copy == some_atomic_read)
{
/* object is still here */
}
unlock(mtx);
return;
}
void
thread_2()
{
struct my_object *m;
m = malloc(sizeof(*m));
if(m == NULL) return;
m-mtx = mtx_alloc();
lock(global);
enqueue_object(m);
unlock(global);
wakeup(global);
sleep(m);
lock(global);
remove_object_from_list_if_present(m);
unlock(global);
lock(mtx);
/*
* XXX increase some refcount atomically
* XXX even if we are locked
*/
unlock(mtx);
free(m);
wakeup(m);
return;
}
If you access the objects via the list/tree/whatsoever most of the time -
refcounting isn't for you.
If you use a single lock, you don't need refcounting. But if you use two
locks, like in this example, and you have more than one source of object
destruction, refcounting is required.
Do you see that more than one thread can be about to destroy the object at the
same time?
--HPS
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: How to do proper locking
On Thursday 04 August 2005 15:53, John Baldwin wrote:
On Thursday 04 August 2005 07:40 am, Hans Petter Selasky wrote:
On Wednesday 03 August 2005 19:21, John Baldwin wrote:
On Tuesday 02 August 2005 06:23 pm, Hans Petter Selasky wrote:
Hi,
I am looking for a safe way to access structures that can be freed.
The solution I am looking for must not:
- hinder scaleability
- lead to use of a single lock
- lead to lock order reversal
There is one more thing. When the structure is freed it should not block
waiting for any reference counts.
crhold() is always called when the caller knows that it has a reference to
cr that can't go away. Thus, in the case of p_ucred, one holds the
associated PROC_LOCK() while doing crhold(). After the crhold() you can
drop the proc lock, so that lock is only held for a very short period of
time. You then have your own reference to the cred structure that is valid
until you call crfree(). In the case of curthread-td_ucred, the reference
is valid until curthread releases it on a cred update at the start of a
syscall, so your reference is valid without needing locks for the life of a
syscall. The key here is that you solve this problem at a higher level,
not within the structure itself.
Yes, this works with mbufs, crhold() and alike, where one doesn't actually
free the object until the refcount reaches zero. But I am speaking about
destroying an object while the refcount is non-zero. Do you see the
difference?
There are two ways to handle this:
1) blocking: wait until the refcount reaches zero.
2) nonblocking: increment some other refcount that the
callback checks before accessing any data.
Do you agree that method 2 is going to:
- avoid deadlock
- avoid use of sx-locks, locks that allow calls to msleep(),
to keep synchronization while destroying objects
with refcounts.
When calling functions like callout_stop(), it is likely that one is holding
a lock to prevent other code from calling callout_start() on the same
callback before callout_stop() has been called. Now, if callout_stop()
sleeps, the lock that is held while calling this function, must allow calls
msleep(). So this must be an sx-lock (see man sx). Now if one routine
sleeps, then any callers of this routine must sleep too. So this affects the
synchronization of the whole system.
Isn't nonblocking operation preferred over blocking operation?
What actual problem are you trying to solve?
Generic callbacks:
- setting up timers. Make sure that callback is never called after that
timer has been stopped.
See callout_stop() and callout_drain(). callout_drain() won't return until
the callout has both been stopped and has finished executing if it was
already in progress.
The callback can be called after that callout_stop() has been called!
You can use _callout_stop_safe(), but the problem is that it will sleep,
waiting for the other thread to finish. The solution I have described,
makes this process non-blocking, which means that one can hold a lock while
calling callout_stop() which must be an advantage.
- setting up devices. Make sure that read/write/ioctl/poll callbacks are
never called after that the device has been freed.
This is managed with refcounts on the dev_t object by devfs. Once you call
destroy_dev() devfs manages the rest.
No, the callback functions read/write/ioctl/poll can be called after that
destroy_dev() has been called.
- setting up interrupts. Make sure that interrupt routine is never called
after that it is been teared down.
bus_teardown_intr() already does this.
I'm sure it is the same here. If the interrupt handler doesn't have its own
lock then, I think the interrupt callback can be called after that
bus_teardown_intr() has been called.
And more. There are lots of situations in the kernel that run into the
same scheme, without a proper solution.
Are there any other places you can think of that don't handle this already?
I am thinking about all the device drivers that create devices in /dev/ that
can be detache, and that really never checks anything before accessing the
softc:
int
ulptread(struct cdev *dev, struct uio *uio, int flags)
{
struct ulpt_softc *sc;
int error;
USB_GET_SC(ulpt, ULPTUNIT(dev), sc);
XXX
XXX here can be an segmentation fault where sc == NULL
XXX
if (sc-sc_dying)
return (EIO);
sc-sc_refcnt++;
error = ulpt_do_read(sc, uio, flags);
if (--sc-sc_refcnt 0)
usb_detach_wakeup(USBDEV(sc-sc_dev));
return (error);
}
The approach FreeBSD's kernel has taken is to solve this problem by
duplicating a lot of checks all over the place for each data structure, but
to solve it in the primitives themselves instead (hence callout_drain(),
bus_teardown_intr(), destroy_dev(), etc.)
Yes, but why are the implementations based on blocking operation, hence this
Re: How to do proper locking
On Thursday 04 August 2005 12:50 pm, Hans Petter Selasky wrote: On Thursday 04 August 2005 15:53, John Baldwin wrote: On Thursday 04 August 2005 07:40 am, Hans Petter Selasky wrote: On Wednesday 03 August 2005 19:21, John Baldwin wrote: On Tuesday 02 August 2005 06:23 pm, Hans Petter Selasky wrote: Hi, I am looking for a safe way to access structures that can be freed. The solution I am looking for must not: - hinder scaleability - lead to use of a single lock - lead to lock order reversal There is one more thing. When the structure is freed it should not block waiting for any reference counts. crhold() is always called when the caller knows that it has a reference to cr that can't go away. Thus, in the case of p_ucred, one holds the associated PROC_LOCK() while doing crhold(). After the crhold() you can drop the proc lock, so that lock is only held for a very short period of time. You then have your own reference to the cred structure that is valid until you call crfree(). In the case of curthread-td_ucred, the reference is valid until curthread releases it on a cred update at the start of a syscall, so your reference is valid without needing locks for the life of a syscall. The key here is that you solve this problem at a higher level, not within the structure itself. Yes, this works with mbufs, crhold() and alike, where one doesn't actually free the object until the refcount reaches zero. But I am speaking about destroying an object while the refcount is non-zero. Do you see the difference? There are two ways to handle this: 1) blocking: wait until the refcount reaches zero. 2) nonblocking: increment some other refcount that the callback checks before accessing any data. Do you agree that method 2 is going to: - avoid deadlock - avoid use of sx-locks, locks that allow calls to msleep(), to keep synchronization while destroying objects with refcounts. Not always. :) When calling functions like callout_stop(), it is likely that one is holding a lock to prevent other code from calling callout_start() on the same callback before callout_stop() has been called. Now, if callout_stop() sleeps, the lock that is held while calling this function, must allow calls msleep(). So this must be an sx-lock (see man sx). Now if one routine sleeps, then any callers of this routine must sleep too. So this affects the synchronization of the whole system. Isn't nonblocking operation preferred over blocking operation? You can call callout_stop() while holding the lock, then do a callout_drain() later when you are prepared to block. This is what I'm doing with ethernet drivers right now that typically call callout_stop in their foo_stop() routine so detach looks like: FOO_LOCK(sc); foo_stop(sc); // calls callout_stop() FOO_UNLOCK(sc); callout_drain(sc-foo_stat_ch); The callback can be called after that callout_stop() has been called! You can use _callout_stop_safe(), but the problem is that it will sleep, waiting for the other thread to finish. The solution I have described, makes this process non-blocking, which means that one can hold a lock while calling callout_stop() which must be an advantage. See above. - setting up devices. Make sure that read/write/ioctl/poll callbacks are never called after that the device has been freed. This is managed with refcounts on the dev_t object by devfs. Once you call destroy_dev() devfs manages the rest. No, the callback functions read/write/ioctl/poll can be called after that destroy_dev() has been called. Nope. Evert call to read/write/ioctl bumps a reference count on the underlying dev_t and destroy_dev() blocks until the reference count drops to zero. - setting up interrupts. Make sure that interrupt routine is never called after that it is been teared down. bus_teardown_intr() already does this. I'm sure it is the same here. If the interrupt handler doesn't have its own lock then, I think the interrupt callback can be called after that bus_teardown_intr() has been called. Nope. The interrupt code checks if the ithread is running and if so it flags the handler for removal and blocks to let the ithread remove the handler and then wake it up. And more. There are lots of situations in the kernel that run into the same scheme, without a proper solution. Are there any other places you can think of that don't handle this already? I am thinking about all the device drivers that create devices in /dev/ that can be detache, and that really never checks anything before accessing the softc: As above, since they use destroy_dev() they are already ok. The approach FreeBSD's kernel has taken is to solve this problem by duplicating a lot of checks all over the place for each data structure, but to solve it in the primitives themselves instead (hence
Re: How to do proper locking
On Thu, Aug 04, 2005 at 06:50:12PM +0200, Hans Petter Selasky wrote: 2) nonblocking: increment some other refcount that the callback checks before accessing any data. I think people usually call this something like a generation count. This sort of scheme used to be used for vnodes in FreeBSD. If you look through the archives for people talking about type-stable storage, you'll probably find some details. David. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: How to do proper locking
On Thursday 04 August 2005 20:15, John Baldwin wrote:
On Thursday 04 August 2005 12:50 pm, Hans Petter Selasky wrote:
On Thursday 04 August 2005 15:53, John Baldwin wrote:
On Thursday 04 August 2005 07:40 am, Hans Petter Selasky wrote:
On Wednesday 03 August 2005 19:21, John Baldwin wrote:
On Tuesday 02 August 2005 06:23 pm, Hans Petter Selasky wrote:
Hi,
I am looking for a safe way to access structures that can be
freed. The solution I am looking for must not:
- hinder scaleability
- lead to use of a single lock
- lead to lock order reversal
There is one more thing. When the structure is freed it should not
block waiting for any reference counts.
crhold() is always called when the caller knows that it has a reference
to cr that can't go away. Thus, in the case of p_ucred, one holds the
associated PROC_LOCK() while doing crhold(). After the crhold() you
can drop the proc lock, so that lock is only held for a very short
period of time. You then have your own reference to the cred structure
that is valid until you call crfree(). In the case of
curthread-td_ucred, the reference is valid until curthread releases it
on a cred update at the start of a syscall, so your reference is valid
without needing locks for the life of a syscall. The key here is that
you solve this problem at a higher level, not within the structure
itself.
Yes, this works with mbufs, crhold() and alike, where one doesn't
actually free the object until the refcount reaches zero. But I am
speaking about destroying an object while the refcount is non-zero. Do
you see the difference?
There are two ways to handle this:
1) blocking: wait until the refcount reaches zero.
2) nonblocking: increment some other refcount that the
callback checks before accessing any data.
Do you agree that method 2 is going to:
- avoid deadlock
- avoid use of sx-locks, locks that allow calls to msleep(),
to keep synchronization while destroying objects
with refcounts.
Not always. :)
When calling functions like callout_stop(), it is likely that one is
holding a lock to prevent other code from calling callout_start() on
the same callback before callout_stop() has been called. Now, if
callout_stop() sleeps, the lock that is held while calling this
function, must allow calls msleep(). So this must be an sx-lock (see man
sx). Now if one routine sleeps, then any callers of this routine must
sleep too. So this affects the synchronization of the whole system.
Isn't nonblocking operation preferred over blocking operation?
You can call callout_stop() while holding the lock, then do a
callout_drain() later when you are prepared to block. This is what I'm
doing with ethernet drivers right now that typically call callout_stop in
their foo_stop() routine so detach looks like:
FOO_LOCK(sc);
foo_stop(sc); // calls callout_stop()
FOO_UNLOCK(sc);
callout_drain(sc-foo_stat_ch);
This is only going to work if you are detaching. But consider the following:
FOO_LOCK(sc);
callout_stop();
callout_start();
FOO_UNLOCK(sc);
Your solution is only spinning halfway around. What I say is that you need
some more parameters passed to callbacks, so that one can check if it has
been cancelled before proceeding.
No, the callback functions read/write/ioctl/poll can be called after that
destroy_dev() has been called.
Nope. Every call to read/write/ioctl bumps a reference count on the
underlying dev_t and destroy_dev() blocks until the reference count drops
to zero.
Yes, I see that dev_refthread() and dev_relthread() are called from
devfs_xxx(), but destroy_dev() will not wait for these refcounts to reach
zero unless the d_purge field is initialized in struct cdevsw. And I
cannot see that prep_cdevs() is setting any default value for this field.
So by default destroy_dev() is non-blocking which is right.
I am a bit lazy today, so I did:
cat `find /usr/src/sys/dev/*` | grep d_purge
And got two hits. This feature is not used at all.
What I want is that the kernel provides some routine that can do locking
based on a structure like below. Also the kernel must provide some
routines to allocate, free and read a refcount.
struct callback_args {
void *sc;
struct mtx *p_mtx;
u_int32_t refcount_copy;
u_int32_t *p_refcount;
};
Then I want routines like callout_reset(), make_dev(),
bus_setup_intr() and so on, to pass these four parameters to the
callback function, either directly, or like a pointer to this structure.
You are just going to move the blocking behavior into contention on your
locks, you aren't going to actually remove it anywhere
Yes, that is correct, but from what I understand it is inherently more
efficient to wait for a mutex of type struct mtx, than it is to use
msleep/wakeup, which is the current way of doing it, according to
Checking sysctl values from within the kernel.
Hello list.
If I want to check a sysctl value from within the kernel (e.g. an KLD),
should I use the system calls described in sysctl(3) ?
If not, what is the propper way to do so ?
And if it is and I want to do error checking e.g:
if((sysctl(name, namelen, val, NULL, 0)) != 0) {
Error in call to sysctl(3) blah blah blah;
}
What would be the correct way to report,stop, c ?
This is propaply depentand on what I'm doing so...
If this is AA I'm sorry for the noize, I have not been able to find
anything on google/archives. (It's late ;)
--
Thordur I. [EMAIL PROTECTED]
Humppa!
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
