Re: [PATCH] copy_from_high_bh

2001-07-08 Thread Andrea Arcangeli

On Sun, Jul 08, 2001 at 07:26:01PM +0100, Mark Hemment wrote:
> On Sun, 8 Jul 2001, Linus Torvalds wrote:
> > On Sun, 8 Jul 2001 [EMAIL PROTECTED] wrote:
> > >
> > >   mm/highmem.c/copy_from_high_bh() blocks interrupts while copying "down"
> > > to a bounce buffer, for writing.
> > >   This function is only ever called from create_bounce() (which cannot
> > > be called from an interrupt context - it may block), and the kmap
> > > 'KM_BOUNCE_WRITE' is only used in copy_from_high_bh().
> >
> > If so it's not just the interrupt disable that is unnecessary, the
> > "kmap_atomic()" should also be just a plain "kmap()", I think.
> 
>   That might eat through kmap()'s virtual window too quickly.

certainly not more quickly than a flood of allocation of highmem pages
in anon mem. I don't see your point. kmap has to work just fine over
there and the __cli/__sti was obviously useless. (should be faster
because we won't need to invlpg at every bounce, but OTOH we'll flush
the whole tlb after the wrap around but as said above if flushing the
tlb too frequently is the issue we should simply improve kmap because
the flood of kmap will happen anyways in the very fast paths regardless
of the bounces)

>   I did think about adding a test to see if the page was already mapped,
> and falling back to kmap_atomic() if it isn't.  That should give the best
> of both worlds?
> 
> Mark


Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



Re: [PATCH] copy_from_high_bh

2001-07-08 Thread Mark Hemment

On Sun, 8 Jul 2001, Linus Torvalds wrote:
> On Sun, 8 Jul 2001 [EMAIL PROTECTED] wrote:
> >
> >   mm/highmem.c/copy_from_high_bh() blocks interrupts while copying "down"
> > to a bounce buffer, for writing.
> >   This function is only ever called from create_bounce() (which cannot
> > be called from an interrupt context - it may block), and the kmap
> > 'KM_BOUNCE_WRITE' is only used in copy_from_high_bh().
>
> If so it's not just the interrupt disable that is unnecessary, the
> "kmap_atomic()" should also be just a plain "kmap()", I think.

  That might eat through kmap()'s virtual window too quickly.

  I did think about adding a test to see if the page was already mapped,
and falling back to kmap_atomic() if it isn't.  That should give the best
of both worlds?

Mark

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



[PATCH] copy_from_high_bh

2001-07-08 Thread markhe

Hi,

  mm/highmem.c/copy_from_high_bh() blocks interrupts while copying "down"
to a bounce buffer, for writing.
  This function is only ever called from create_bounce() (which cannot
be called from an interrupt context - it may block), and the kmap
'KM_BOUNCE_WRITE' is only used in copy_from_high_bh().

  Therefore, the comment in copy_from_high_bh() is wrong, and the
__cli()/__sti() unnecessary.

  Patch against 2.4.6 below.

Mark

---

diff -urN -X dontdiff linux-2.4.6/mm/highmem.c cli-2.4.6/mm/highmem.c
--- linux-2.4.6/mm/highmem.cFri Jun 29 23:17:34 2001
+++ cli-2.4.6/mm/highmem.c  Sun Jul  8 15:50:04 2001
@@ -182,20 +182,12 @@
 {
struct page *p_from;
char *vfrom;
-   unsigned long flags;

p_from = from->b_page;

-   /*
-* Since this can be executed from IRQ context, reentrance
-* on the same CPU must be avoided:
-*/
-   __save_flags(flags);
-   __cli();
vfrom = kmap_atomic(p_from, KM_BOUNCE_WRITE);
memcpy(to->b_data, vfrom + bh_offset(from), to->b_size);
kunmap_atomic(vfrom, KM_BOUNCE_WRITE);
-   __restore_flags(flags);
 }

 static inline void copy_to_high_bh_irq (struct buffer_head *to,


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



[PATCH] copy_from_high_bh

2001-07-08 Thread markhe

Hi,

  mm/highmem.c/copy_from_high_bh() blocks interrupts while copying down
to a bounce buffer, for writing.
  This function is only ever called from create_bounce() (which cannot
be called from an interrupt context - it may block), and the kmap
'KM_BOUNCE_WRITE' is only used in copy_from_high_bh().

  Therefore, the comment in copy_from_high_bh() is wrong, and the
__cli()/__sti() unnecessary.

  Patch against 2.4.6 below.

Mark

---

diff -urN -X dontdiff linux-2.4.6/mm/highmem.c cli-2.4.6/mm/highmem.c
--- linux-2.4.6/mm/highmem.cFri Jun 29 23:17:34 2001
+++ cli-2.4.6/mm/highmem.c  Sun Jul  8 15:50:04 2001
@@ -182,20 +182,12 @@
 {
struct page *p_from;
char *vfrom;
-   unsigned long flags;

p_from = from-b_page;

-   /*
-* Since this can be executed from IRQ context, reentrance
-* on the same CPU must be avoided:
-*/
-   __save_flags(flags);
-   __cli();
vfrom = kmap_atomic(p_from, KM_BOUNCE_WRITE);
memcpy(to-b_data, vfrom + bh_offset(from), to-b_size);
kunmap_atomic(vfrom, KM_BOUNCE_WRITE);
-   __restore_flags(flags);
 }

 static inline void copy_to_high_bh_irq (struct buffer_head *to,


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



Re: [PATCH] copy_from_high_bh

2001-07-08 Thread Mark Hemment

On Sun, 8 Jul 2001, Linus Torvalds wrote:
 On Sun, 8 Jul 2001 [EMAIL PROTECTED] wrote:
 
mm/highmem.c/copy_from_high_bh() blocks interrupts while copying down
  to a bounce buffer, for writing.
This function is only ever called from create_bounce() (which cannot
  be called from an interrupt context - it may block), and the kmap
  'KM_BOUNCE_WRITE' is only used in copy_from_high_bh().

 If so it's not just the interrupt disable that is unnecessary, the
 kmap_atomic() should also be just a plain kmap(), I think.

  That might eat through kmap()'s virtual window too quickly.

  I did think about adding a test to see if the page was already mapped,
and falling back to kmap_atomic() if it isn't.  That should give the best
of both worlds?

Mark

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



Re: [PATCH] copy_from_high_bh

2001-07-08 Thread Andrea Arcangeli

On Sun, Jul 08, 2001 at 07:26:01PM +0100, Mark Hemment wrote:
 On Sun, 8 Jul 2001, Linus Torvalds wrote:
  On Sun, 8 Jul 2001 [EMAIL PROTECTED] wrote:
  
 mm/highmem.c/copy_from_high_bh() blocks interrupts while copying down
   to a bounce buffer, for writing.
 This function is only ever called from create_bounce() (which cannot
   be called from an interrupt context - it may block), and the kmap
   'KM_BOUNCE_WRITE' is only used in copy_from_high_bh().
 
  If so it's not just the interrupt disable that is unnecessary, the
  kmap_atomic() should also be just a plain kmap(), I think.
 
   That might eat through kmap()'s virtual window too quickly.

certainly not more quickly than a flood of allocation of highmem pages
in anon mem. I don't see your point. kmap has to work just fine over
there and the __cli/__sti was obviously useless. (should be faster
because we won't need to invlpg at every bounce, but OTOH we'll flush
the whole tlb after the wrap around but as said above if flushing the
tlb too frequently is the issue we should simply improve kmap because
the flood of kmap will happen anyways in the very fast paths regardless
of the bounces)

   I did think about adding a test to see if the page was already mapped,
 and falling back to kmap_atomic() if it isn't.  That should give the best
 of both worlds?
 
 Mark


Andrea
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/