Thanks Andi, I know you have sent most of this before, things are a just
a 'leetle bit' hectic around here right now, so things are taking longer
than they otherwise would. The problem with the code which does some
xfs memory allocation failure detection is that you can never get to
all of them, this is why I have never checked in the stuff about
seeing a NULL and doing an error return. There are also places in xfs
where failure is not an option - once a transaction has dirtied
metadata there is no turning back. So really the only option which will
fly long term is making sure memory allocations do not return failure
when they get back up to xfs proper. I do have some other ideas it is
just a matter of finding the time.
Steve
>
> I ran into a few problems with XFS running out of memory and crashing
> and did a few changes to fix the most obvious problems. The only tricky
> bit is the ENOMEM return in xfs_trans_read_buf(); there may be some
> callers that handle NULL correctly but most seem not to when the error
> return is zero. The others are straight forward. I replace the panics
> in kmem.c with random retries with printks to give it some chance to
> recover. Sometimes it deadlocks unfortunately (probably because too
> many locks are held), but in other cases it recovers and that's better
> than a panic. I also fixed oom checking in some other easy cases.
>
> This patch also fixes an unrelated bug -- pagebuf sometimes sets
> pb_error to negative errno when the rest of XFS expects positive here.
>
> -Andi
>
>
> --- linux-xfs/fs/pagebuf/page_buf.c-XFSMEM Tue Aug 14 01:12:43 2001
> +++ linux-xfs/fs/pagebuf/page_buf.c Tue Aug 14 20:43:57 2001
> @@ -1055,6 +1055,8 @@
> int pagebuf_geterror( /* return buffer error */
> page_buf_t * pb) /* buffer */
> {
> + if (!pb)
> + return ENOMEM;
> return (pb->pb_error);
> }
>
> @@ -1200,7 +1202,7 @@
> page = bh->b_page;
> if (!test_bit(BH_Uptodate, &bh->b_state)) {
> set_bit(PG_error, &page->flags);
> - pb->pb_error = -EIO;
> + pb->pb_error = EIO;
> }
>
> unlock_buffer(bh);
> @@ -1221,7 +1223,7 @@
> page = bh->b_page;
> if (!test_bit(BH_Uptodate, &bh->b_state)) {
> set_bit(PG_error, &page->flags);
> - pb->pb_error = -EIO;
> + pb->pb_error = EIO;
> }
>
> unlock_buffer(bh);
> @@ -1244,7 +1246,7 @@
> page = bh->b_page;
> if (!test_bit(BH_Uptodate, &bh->b_state)) {
> set_bit(PG_error, &page->flags);
> - pb->pb_error = -EIO;
> + pb->pb_error = EIO;
> }
>
> unlock_buffer(bh);
> --- linux-xfs/fs/xfs/linux/xfs_vfs.c-XFSMEM Tue Aug 14 01:12:44 2001
> +++ linux-xfs/fs/xfs/linux/xfs_vfs.c Tue Aug 14 04:51:07 2001
> @@ -52,6 +52,8 @@
> vfs_t *vfsp;
>
> vfsp = kmalloc(sizeof(vfs_t), GFP_KERNEL);
> + if (!vfsp)
> + return NULL;
> memset(vfsp, 0, sizeof(vfs_t));
> ASSERT(vfsp);
> VFS_INIT(vfsp);
> --- linux-xfs/fs/xfs/linux/xfs_super.c-XFSMEM Tue Aug 14 01:12:44 2001
> +++ linux-xfs/fs/xfs/linux/xfs_super.c Tue Aug 14 02:06:09 2001
> @@ -357,6 +357,8 @@
> /* Set up the vfs_t structure */
>
> vfsp = vfs_allocate();
> + if (!vfsp)
> + return NULL;
>
> if (sb->s_flags & MS_RDONLY)
> vfsp->vfs_flag |= VFS_RDONLY;
> @@ -364,6 +366,11 @@
> /* Setup up the cvp structure */
>
> cip = (struct inode *)kmem_alloc(sizeof(struct inode),0);
> + if (!cip) {
> + vfs_deallocate(vfsp);
> + return NULL;
> + }
> +
> bzero(cip, sizeof(*cip));
>
> atomic_set(&cip->i_count, 1);
> --- linux-xfs/fs/xfs/xfs_buf.h-XFSMEM Tue Aug 14 01:44:21 2001
> +++ linux-xfs/fs/xfs/xfs_buf.h Tue Aug 14 01:50:01 2001
> @@ -238,6 +238,9 @@
>
> static inline void xfs_buf_relse(page_buf_t *bp)
> {
> + if (!bp)
> + return;
> +
> if (bp->pb_relse == NULL)
> pagebuf_unlock(bp);
>
> --- linux-xfs/fs/xfs/xfs_trans_buf.c-XFSMEM Tue Aug 14 01:12:46 2001
> +++ linux-xfs/fs/xfs/xfs_trans_buf.c Tue Aug 14 20:28:33 2001
> @@ -295,6 +295,9 @@
> */
> if (tp == NULL) {
> bp = xfs_buf_read_flags(target, blkno, len, flags);
> + if (!bp)
> + return XFS_ERROR(ENOMEM);
> +
> if ((bp != NULL) && (XFS_BUF_GETERROR(bp) != 0)) {
> xfs_ioerror_alert("xfs_trans_read_buf", mp,
> target->dev, blkno);
> --- linux-xfs/fs/xfs_support/kmem.c-XFSMEM Tue Aug 14 01:12:46 2001
> +++ linux-xfs/fs/xfs_support/kmem.c Tue Aug 14 01:51:14 2001
> @@ -47,6 +47,15 @@
> #define GFP_NOFS GFP_BUFFER
> #endif
>
> +/* random generator stolen from net/core/utils.c. */
> +static unsigned long xfs_rand_seed = 152L;
> +
> +unsigned long xfs_random(void)
> +{
> + xfs_rand_seed=xfs_rand_seed*69069L+1;
> + return xfs_rand_seed^jiffies;
> +}
> +
> static __inline__ unsigned int flag_convert(int flags)
> {
> if (flags & KM_NOSLEEP) return GFP_ATOMIC;
> @@ -79,8 +88,13 @@
> rval = kmalloc(size, flag_convert(flags));
> }
>
> - if (rval || (flags & KM_NOSLEEP))
> - return rval;
> + if (!rval) {
> + if ((flags & KM_NOSLEEP) == 0) {
> + printk(KERN_ERR "xfs [%d]: failed to allocate %d bytes.
> \n", current->pid, size);
> + return NULL;
> + }
> + } else
> + return rval;
>
> /*
> * KM_SLEEP callers don't expect a failure
> @@ -93,8 +107,16 @@
> }
>
> rval = __vmalloc(size, flag_convert(flags), PAGE_KERNEL);
> - if (!rval && (flags & KM_SLEEP))
> - panic("kmem_alloc: NULL memory on KM_SLEEP request!");
> + if (!rval) {
> + if (flags & KM_SLEEP) {
> + printk(KERN_ERR "xfs [%d]: failed to allocate %d bytes.
> waiting and retry.\n",
> + current->pid, size);
> + delay(5 + xfs_random()%10);
> + shrink = DEF_PRIORITY;
> + goto repeat;
> + }
> + printk(KERN_ERR "xfs [%d]: failed to allocate %d bytes.\n", cur
> rent->pid, size);
> + }
>
> return rval;
> }
> @@ -157,8 +179,11 @@
> repeat:
> ptr = kmem_cache_alloc(zone, flag_convert(flags));
>
> - if (ptr || (flags & KM_NOSLEEP))
> + if (ptr || (flags & KM_NOSLEEP)) {
> + if (!ptr)
> + printk(KERN_ERR "xfs [%d]: failed to allocate from zone
> .\n", current->pid);
> return ptr;
> + }
>
> /*
> * KM_SLEEP callers don't expect a failure
> @@ -170,8 +195,13 @@
> goto repeat;
> }
>
> - if (flags & KM_SLEEP)
> - panic("kmem_zone_alloc: NULL memory on KM_SLEEP request!");
> + if (flags & KM_SLEEP) {
> + printk(KERN_ERR "xfs [%d]: failed to allocate from zone. waitin
> g and retry.\n",
> + current->pid);
> + delay(5 + xfs_random()%10);
> + shrink = DEF_PRIORITY;
> + goto repeat;
> + }
>
> return NULL;
> }
> @@ -185,21 +215,29 @@
> repeat:
> ptr = kmem_cache_zalloc(zone, flag_convert(flags));
>
> - if (ptr || (flags & KM_NOSLEEP))
> + if (ptr || (flags & KM_NOSLEEP)) {
> + if (!ptr)
> + printk(KERN_ERR "xfs [%d]: failed to allocate from zone
> .\n", current->pid);
> return ptr;
> + }
>
> /*
> * KM_SLEEP callers don't expect a failure
> */
> if (shrink) {
> kmem_shake();
> -
> +
> shrink--;
> goto repeat;
> }
>
> - if (flags & KM_SLEEP)
> - panic("kmem_zone_zalloc: NULL memory on KM_SLEEP request!");
> + if (flags & KM_SLEEP) {
> + printk(KERN_ERR "xfs [%d]: failed to allocate from zone. waitin
> g and retry.\n",
> + current->pid);
> + delay(5 + xfs_random()%10);
> + shrink = DEF_PRIORITY;
> + goto repeat;
> + }
>
> return NULL;
> }
|